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

Submitted by:  None
Submitted on:  Wed 04 May 2016 06:15:52 PM UTC  
 
Category: Programmer interfaceSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: Daiki Ueno <ueno>
Open/Closed: Closed

Fri 13 May 2016 02:01:23 AM UTC, comment #4:

http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=d8cd505c692ec0eb647135fef23f7f2b037f7571

Daiki Ueno <ueno>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project 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>
Project AdministratorIn charge of this item.
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 -unavailable- (or simply the -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

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 27 Nov 2016 12:12:37 PM UTChaibleCategoryNone=>Programmer interface
      Assigned toNone=>ueno
    Fri 13 May 2016 02:01:23 AM UTCuenoStatusNone=>Fixed
      PrivacyPrivate=>Public
      Open/ClosedOpen=>Closed
    Mon 09 May 2016 03:27:56 AM UTCuenoCarbon-Copy-=>Added -unavailable-

    Back to the top


    Powered by Savane 3.1-cleanup1