Re: [PATCH 36/38] trace-cmd record: check the length of the protocol version received

On 18/07/2024 04:11, Steven Rostedt wrote:
> On Wed,  5 Jun 2024 15:40:51 +0200
> "Jerome Marchand" <[email protected]> wrote:
> 
>> In check_protocol_version we compare the protocol version string with
>> the expected one ("V3") with memcmp(). The received string could be
>> longer than the constant string used for the comparison. That could
>> lead to out of range access.
>>
>> Check that the received protocol version is not too long.
>>
>> Fixes a OVERRUN error (CWE-119)
>>
>> Signed-off-by: Jerome Marchand <[email protected]>
>> ---
>>   tracecmd/trace-record.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
>> index dc3e5285..c3118546 100644
>> --- a/tracecmd/trace-record.c
>> +++ b/tracecmd/trace-record.c
>> @@ -3810,7 +3810,7 @@ static void check_protocol_version(struct tracecmd_msg_handle *msg_handle)
>>   		msg_handle->version = V1_PROTOCOL;
>>   		tracecmd_plog("Use the v1 protocol\n");
>>   	} else {
>> -		if (memcmp(buf, "V3", n) != 0)
>> +		if (n > 3 || memcmp(buf, "V3", n) != 0)
>>   			die("Cannot handle the protocol %s", buf);
> 
> Actually, we may add more to it, so this should be:
> 
> 		if (n < 3 || memcmp(buf, "V3", 3) != 0)

That's definitely more future proof. I'll send an updated version.

Jerome

> 
> -- Steve
> 
>>   		/* OK, let's use v3 protocol */
>>   		write(fd, V3_MAGIC, sizeof(V3_MAGIC));
>