From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 28 May 2024 14:49:26 GMT
Subject: Re: [PATCH] fix: prevent date underflow when using positive timezone offset
Message-ID: <0c2addd0-104d-4142-969a-500aab39c90e@gmail.com>
In-Reply-To: <ZlXkvEeR-PgZMitx@tanuki>

On 28/05/2024 15:05, Patrick Steinhardt wrote:
> On Mon, May 27, 2024 at 09:17:06AM +0000, darcy via GitGitGadget wrote:
>> From: darcy <acednes@gmail.com>
>> diff --git a/date.c b/date.c
>> index 7365a4ad24f..8388629f267 100644
>> --- a/date.c
>> +++ b/date.c
>> @@ -908,7 +908,7 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
>>   			match = match_alpha(date, &tm, offset);
>>   		else if (isdigit(c))
>>   			match = match_digit(date, &tm, offset, &tm_gmt);
>> -		else if ((c == '-' || c == '+') && isdigit(date[1]))
>> +		else if ((c == '-' || c == '+') && isdigit(date[1]) && tm.tm_hour != -1)
>>   			match = match_tz(date, offset);
> 
> Without having a deep understanding of the code I don't quite see the
> connection between this change and the problem description. Is it
> necessary? If so, it might help to explain why it's needed in the commit
> message or in the code.

I was wondering about this change too

>>   		if (!match) {
>> @@ -937,8 +937,13 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
>>   		}
>>   	}
>>   
>> -	if (!tm_gmt)
>> +	if (!tm_gmt) {
>> +		if (*offset > 0 && *offset * 60 > *timestamp) {
>> +			return -1;
>> +		}
> 
> Nit: we don't add curly braces around one-line conditional bodies.
> 
> This change here is the meat of it and looks like I'd expect.
> 
>>   		*timestamp -= *offset * 60;

Do we also want to check for overflows in the other direction (a large 
timestamp with a negative timezone offset)?

Best Wishes

Phillip