Re: [PATCH 2/2] smb: client: fix potential OOB in smb2_dump_detail()

[Paulo Alcantara <pc@manguebit.com>]

Maximilian Heyne <mheyne@amazon.de> writes:

> On Sat, Dec 16, 2023 at 01:10:05AM -0300, Paulo Alcantara wrote:
>> Validate SMB message with ->check_message() before calling
>> ->calc_smb_size().
>> 
>> This fixes CVE-2023-6610.
>> 
>> Reported-by: j51569436@gmail.com
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218219
>> Signed-off-by: Paulo Alcantara <pc@manguebit.com>
>> ---
>>  fs/smb/client/smb2ops.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
>> index 62b0a8df867b..66b310208545 100644
>> --- a/fs/smb/client/smb2ops.c
>> +++ b/fs/smb/client/smb2ops.c
>> @@ -403,8 +403,10 @@ smb2_dump_detail(void *buf, struct TCP_Server_Info *server)
>>  	cifs_server_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Mid: %llu Pid: %d\n",
>>  		 shdr->Command, shdr->Status, shdr->Flags, shdr->MessageId,
>>  		 shdr->Id.SyncId.ProcessId);
>> -	cifs_server_dbg(VFS, "smb buf %p len %u\n", buf,
>> -		 server->ops->calc_smb_size(buf));
>> +	if (!server->ops->check_message(buf, server->total_read, server)) {
>
> Why is this check negated? Sorry for this stupid question. I'm not
> familiar with this code but only stumbled upon this commit due to a
> CVE.

Because smb2_check_message() returns 0 for a valid SMB response and then
it would be safe to call ->calc_smb_size() as we know the header has a
valid Command (< NUMBER_OF_SMB2_COMMANDS).