Stricter rules when parsing time values to avoid UBSAN error

[kevinbackhouse]

This fixes a UBSAN error found by OSS-Fuzz: https://issues.oss-fuzz.com/issues/392928817
The error message is:

/src/exiv2/src/value.cpp:975:43: runtime error: negation of -2147483648 cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself

It happens here:

exiv2/src/value.cpp

Line 975 in 3b58bda

time_.tzMinute = time_.tzHour < 0 ? -minute : minute;

The integer overflow is harmless, but I have fixed it by making the parsing rules stricter.

[kevinbackhouse]

@Mergifyio backport 0.28.x

[mergify]

backport 0.28.x

✅ Backports have been created

Details

• #3151 Stricter rules when parsing time values to avoid UBSAN error (backport #3148) has been created for branch 0.28.x

[kmilos]

Could limit this (and one above) even more from -12 to 14 strictly speaking, but I guess this is just to keep the fuzzer from going to crazy...

[neheb]

@kevinbackhouse woun't it be better to use strtoul for some of these?

[kevinbackhouse]

@kevinbackhouse woun't it be better to use strtoul for some of these?

I don't think that would help, because they get written into a struct with int32_t fields:

exiv2/include/exiv2/value.hpp

Lines 1003 to 1009 in b917b34

	struct Time {
	int32_t hour; //!< Hour
	int32_t minute; //!< Minute
	int32_t second; //!< Second
	int32_t tzHour; //!< Hours ahead or behind UTC
	int32_t tzMinute; //!< Minutes ahead or behind UTC
	};

[neheb]

So a cast would turn negatives to 0?

[kevinbackhouse]

So a cast would turn negatives to 0?

What I mean is: if we use strtoul then we will have to cast from unsigned long to int32_t, which creates other opportunities for integer overflows, so it won't solve the problem of UBSAN errors.

Of course these integer overflows are completely harmless, and they don't happen on legitimate image files, so this PR is purely about silencing an irrelevant UBSAN error.