[PATCH] iconvdata: Fix invalid pointer arithmetic in ANSI_X3.110 module

[Florian Weimer]

* Collin Funk:

> Florian Weimer <fweimer@redhat.com> writes:
>
>> The expression inptr + 1 can technically be invalid: if inptr == inend,
>> inptr may point one element past the end of an array.
>>
>> ---
>>  iconvdata/ansi_x3.110.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/iconvdata/ansi_x3.110.c b/iconvdata/ansi_x3.110.c
>> index c5506b13b8..94e6e6b745 100644
>> --- a/iconvdata/ansi_x3.110.c
>> +++ b/iconvdata/ansi_x3.110.c
>> @@ -407,7 +407,7 @@ static const char from_ucs4[][2] =
>>  	   is also available.  */					      \
>>  	uint32_t ch2;							      \
>>  									      \
>> -	if (inptr + 1 >= inend)						      \
>> +	if (inend - inptr <= 1)						      \
>>  	  {								      \
>>  	    /* The second character is not available.  */		      \
>>  	    result = __GCONV_INCOMPLETE_INPUT;				      \
>>
>> base-commit: 15de57024611ed6e668acbc440c5e360b0543374
>
> Just curious, was this found with -Wstrict-overflow (I think that is the
> right one)? Might be worth mentioning the warning options used in the
> commit message if so.

No, there's actually a funnt story behind it.

We're moving the ANSI_X3.110 module between RPM packages in RHEL 9.
This causes file conflicts between parallel installable debuginfo
packages packages because the build ID symbolic link under
/usr/lib/debug is in both packages, but points to debuginfo files in
different locations:

<https://gitlab.com/redhat/centos-stream/rpms/glibc/-/merge_requests/449#note_2920111549>

Making a dummy change to the converter at the same time we move it
between RPM packages changes the /usr/lib/debug path because it's based
on the build ID.  So I started to look for a small bug to fix in the
converter.  I was first wondering if we should fix the mapping for 0x23
and 0x24 (it's strange that an U.S. character set doesn't have the
dollar sign at the expected place, but uses a multibyte sequence for
it), but it turns out it's hard to find authoritative information about
the character set.  Doing the pointer comparison the wrong way is quite
common under iconvdata/, so eventually spotted it.  It's a real bug, but
I also hope that it makes the RPM file conflict go away once we
backport it.

Thanks,
Florian