Re: [PATCH 01/11] mingw: avoid accessing uninitialized memory in `is_executable()`

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 16, 2022, 20:13 UTC
Message-ID: <xmqqmtecs6ws.fsf@gitster.g>
In-Reply-To: <98ff788a-d621-cf3f-aed5-66ae763b381f@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 11 quoted lines

> Am 16.06.22 um 01:35 schrieb Johannes Schindelin via GitGitGadget:
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> On Windows, we open files we suspect might be scripts, read the first
>> two bytes, and see whether they indicate a hash-bang line. We do not
>> initialize the byte _after_ those two bytes, therefore `strcmp()` is
>> inappropriate here.
>
> Hmm, but buf _is_ initialized fully?  Line 149:
>
>         char buf[3] = { 0 };

Ahh, yeah, that changes the landscape quite a bit.

We explicitly ask to read 2 bytes and look at the buf[] when read says it read 2 bytes, so this is another false positive X-<.

Show 13 quoted lines

>> diff --git a/run-command.c b/run-command.c
>> index 14f17830f51..2ba38850b4d 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -154,7 +154,7 @@ int is_executable(const char *name)
>>  		n = read(fd, buf, 2);
>>  		if (n == 2)
>>  			/* look for a she-bang */
>> -			if (!strcmp(buf, "#!"))
>> +			if (!memcmp(buf, "#!", 2))
>>  				st.st_mode |= S_IXUSR;
>>  		close(fd);
>>  	}

We can update the variable to

	char buf[2];

to match the updated code, I guess. The fewer bytes we use on stack, the better ;-).