bugGNU gettext - Bugs: bug #47847, Undefined behavior...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #47847: Undefined behavior [use-after-free] possible in libgettext

Submitter:  None
Submitted:  Wed 04 May 2016 06:15:52 PM UTC
   
 
Category:  Programmer interface Severity:  3 - Normal
Item Group:  None Status:  Fix Released
Privacy:  Public Assigned to:  ueno
Open/Closed:  Closed

Fri 13 May 2016 02:01:23 AM UTC, comment #4: 
Daiki Ueno <ueno>
Group administrator
Mon 09 May 2016 08:51:55 AM UTC, comment #3: 

Thanks for the comment, Bruno.

The reasoning sounds convincing, but I'm a bit confused that there is no such path in the original code.  ISO C 6.2.4 also says: "The result of attempting to indirectly access an object with automatic storage duration from a thread other than the one with which the object is associated is implementation-defined", but I neither see a possibility of this.

So far, the more I think of this, the more it seems like a false positive (and if so, perhaps we could add an annotation instead to suppress the warning).

Daiki Ueno <ueno>
Group administrator
Mon 09 May 2016 07:14:09 AM UTC, comment #2: 

IMO the sentence "The value of a pointer becomes indeterminate when
the object it points to reaches the end of its lifetime." (ISO C 6.2.4) means that you should not compare pointers to local variables which have gone out of scope: The function

int foo (int x)
{
  int *pa, *pb;
  { int a = x; pa = &a; }
  { int b = x; pb = &b; }
  return pa == pb;
}

can return 0 or 1, depending on compiler optimizations. This explains the coverity warning.

Bruno Haible <haible>
Group administrator
Mon 09 May 2016 03:27:56 AM UTC, comment #1: 

Thanks for pointing that out.

> A reasonable fix is to convert the pointers to be compared to uintptr while both pointers are still valid


It sounds like a good idea, but I doubt we can always assume that uintptr_t is available.  So, I have created a patch to fix it by postponing free:
https://lists.gnu.org/archive/html/bug-gnulib/2016-05/msg00016.html

Does it make sense?

Daiki Ueno <ueno>
Group administrator
Wed 04 May 2016 06:15:52 PM UTC, original submission:  

A Coverity warning for KDE's "KI18n" framework for internationalization noted that a piece of code derived from libgettext uses the indeterminate value of a free()'d pointer in a comparison with a different pointer (but only when _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS is not defined).

The bug appears to still be present in the gettext upstream; see http://git.savannah.gnu.org/cgit/gettext.git/tree/gnulib-local/lib/gettext.h#n236 and http://git.savannah.gnu.org/cgit/gettext.git/tree/gnulib-local/lib/gettext.h#n282

According to the C standard, any use of the 'indeterminate' value of a free()'d pointer is UB, no matter how innocuous (see entry 10 from https://www.securecoding.cert.org/confluence/display/c/CC.+Undefined+Behavior, which quotes the standard's Annex J.2 directly).

A reasonable fix is to convert the pointers to be compared to uintptr while both pointers are still valid, and then do the comparisons needed against the uintptrs. Alternately, one could use a boolean flag to save the result of the comparison while the pointers are both valid, free the memory if needed, and then branch based on the comparison result.

Either of these would fix the UB while maintaining the right behavior.

I will fix in KDE but wanted to make sure upstream was warned. Please contact at -email is unavailable- (or simply the -email is unavailable- mailing list) if you need to reach out for more information. I have checked the public bug listing and did not see an existing report, my apologies if this is a dupe.

Regards,
 - Michael Pyne

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by haible (Posted a comment)
  • -email is unavailable- added by ueno (Posted a comment)
  • -email is unavailable- added by ueno
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-01-04 haible StatusFixed Fix Released
    2016-11-27 haible CategoryNone Programmer interface
        Assigned toNone ueno
    2016-05-13 ueno StatusNone Fixed
        PrivacyPrivate Public
        Open/ClosedOpen Closed
    2016-05-09 ueno Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code