Re: [RFC] newlib/libc/include/langinfo.h: nl_langinfo enum off by one error causing pointer overwrite

On 2024-08-23 02:39, Corinna Vinschen wrote:
> On Aug 22 15:25, Brian Inglis wrote:
>> On 2024-08-22 13:13, Corinna Vinschen wrote:
>>> On Aug 22 11:14, Brian Inglis wrote:
>>>> As a result, the underlying pointers in the pointer array are off by one in
>>>> the wide messages fields, and the _NL_COLLATE_CODESET pointer clobbers what
>>>> was stored in the _NL_MESSAGES_WNOSTR position in the pointer array.
>>>
>>> Yes, youre' right, there's an off by one.  But this can't be fixed by
>>> chaning nl_item values.
>>>
>>> The culprit is actually in nl_langinfo.c.  The internal nl_ext array
>>> erronously contains an entry for lc_monetary_T::codeset which is not
>>> part of the _NL_LOCALE_EXTENDED entries.  This moves the wide char
>>> yes/no strings accidentally by one.  The patch is simple:
>>>
>>> diff --git a/newlib/libc/locale/nl_langinfo.c b/newlib/libc/locale/nl_langinfo.c
>>> index c34a7d131376..4477d833bec1 100644
>>> --- a/newlib/libc/locale/nl_langinfo.c
>>> +++ b/newlib/libc/locale/nl_langinfo.c
>>> @@ -160,7 +160,6 @@ static struct _nl_item_t
>>>      _NLITEM (monetary, wmon_thousands_sep),
>>>      _NLITEM (monetary, wpositive_sign),
>>>      _NLITEM (monetary, wnegative_sign),
>>> -  _NLITEM (messages, codeset),
>>>      _NLITEM (messages, wyesexpr),
>>>      _NLITEM (messages, wnoexpr),
>>>      _NLITEM (messages, wyesstr),
>>>
>>>> If __HAVE_LOCALE_INFO__ is not defined, then _NL_MESSAGES_CODESET is
>>>> also not defined, so it is unclear if _NL_MESSAGES_CODESET should be
>>>> defined to _NL_CTYPE_CODESET if neither __HAVE_LOCALE_INFO__ nor
>>>> __HAVE_LOCALE_INFO_EXTENDED__ are defined, or added as another field
>>>> depending on those definitions?
>>>
>>> No, see above.  There is no definition of _NL_CTYPE_CODESET, only
>>> _NL_CTYPE_CODESET_NAME == CODESET.  _NL_MESSAGES_CODESET is exactly
>>> where it has to be. If you start moving _NL_* values around, you *will*
>>> break backward compatibility with existing executables.
>>> [...]
>>
>> But aren't those entries populated from Windows, where the codeset is copied in
>> winsup/cygwin/nlsfuncs.cc(__set_lc_messages_from_win)
>>
>> https://cygwin.com/git?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/nlsfuncs.cc#l1114
>>
>> into newlib/libc/locale/setlocale.h(lc_messages_t):
>>
>> https://cygwin.com/git?p=newlib-cygwin.git;a=blob;f=newlib/libc/locale/setlocale.h#l166
>>
>> which includes codeset?
> 
> No.  First of all, apart from fetching LANG from the windows locale
> at startup (see /etc/profile.d/lang.*), the codeset is maintained by
> Cygwin/newlib alone, via LC_* environment and setlocale() calls in
> the application.
> 
> Also, Windows has no GetLocaleInfo call for fetching locale-specific
> yes/no strings.  I. e., there's no equivalent for LC_MESSAGES.
> 
> Therefore Cygwin fetches the information from static information
> maintained in winsup/cygwin/local_includes/lc_msg.h.
> 
> If you look closely into __set_lc_messages_from_win(), you'll
> notice that the __get_rfc5646_from_locale() function is called
> from __set_lc_messages_from_win() *only* to check if a matching
> Windows locale exists.  This is a pure sanity check.  After that,
> any further information is taken from the lc_msg struct defined
> in lc_msg.h, which in turn has been fetched from Linux.  See
> winsup/cygwin/linux-locale-helpers.
> 
> Does that make sense?

Yes - I dug into all those sources and that is not any issue.

But __set_lc_messages_from_win() then adds the codeset field before the wide 
character fields, as with the other Cygwin functions, so would it not be more 
consistent and simpler to just add an enum entry for the codeset, called 
something unique including ...CODESET..., keeping the entry and realigning tha 
last few indexes with the items, as nobody else has noticed any issue in the 
last 15 years ;^>

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry