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

From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 01/11] mingw: avoid accessing uninitialized memory in `is_executable()`
Date: Thu, 16 Jun 2022 13:13:55 -0700	[thread overview]
Message-ID: <xmqqmtecs6ws.fsf@gitster.g> (raw)
In-Reply-To: <98ff788a-d621-cf3f-aed5-66ae763b381f@web.de> (=?utf-8?Q?=22R?= =?utf-8?Q?en=C3=A9?= Scharfe"'s message of "Thu, 16 Jun 2022 21:53:28 +0200")

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

> 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-<.

>> 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 ;-).