Bug 14277 - Pointer used after free'd
Summary: Pointer used after free'd
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.15
: P2 normal
Target Milestone: ---
Assignee: law
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-21 16:05 UTC by law
Modified: 2014-06-18 04:35 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security+


Attachments
Potential fix (456 bytes, patch)
2012-06-21 16:05 UTC, law
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description law 2012-06-21 16:05:39 UTC
Created attachment 6463 [details]
Potential fix

dcigettext.c has a very clear use-after-free problem which can result in a user process getting a segfault or other error.

                      newmem = (transmem_block_t *) realloc (transmem_list,
                                                             freemem_size);
# ifdef _LIBC
                      if (newmem != NULL)
                        transmem_list = transmem_list->next;
                      else
                        {
                          struct transmem_list *old = transmem_list;

                          transmem_list = transmem_list->next;
                          free (old);
                        }
# endif

If the call to realloc requires transmem_list's memory to be moved, then the prior pointer is free'd.  Most of the time this doesn't cause a noticeable problem because the memory is still mapped and the dereference in transmem_list->next shortly after the realloc call works fine (this is still wrong obviously).

However, if transmem_list was allocated by mmap (because it was large), when the original pointer is free'd as a result of the realloc call, the old memory gets ummapped.  With the memory now unmapped, the transmem_list->next dereference of the old pointer fails with a segfault.

I don't have a good testcase -- the one provided to me hasn't tripped in about 12 hours of running or in shorter runs with differing values of M_MMAP_THRESHOLD and may contain confidential information.   Hopefully the analysis above is clear enough to show this code is clearly broken.


Attached is a potential fix for this problem.
Comment 1 joseph@codesourcery.com 2012-06-21 16:21:12 UTC
On Thu, 21 Jun 2012, law at redhat dot com wrote:

> I don't have a good testcase -- the one provided to me hasn't tripped in about
> 12 hours of running or in shorter runs with differing values of
> M_MMAP_THRESHOLD and may contain confidential information.   Hopefully the
> analysis above is clear enough to show this code is clearly broken.

Does use of M_PERTURB (to cause free to overwrite freed memory - I don't 
actually know if it works for realloc as well) make it easier to reproduce 
the problem?
Comment 2 law 2012-06-21 18:00:06 UTC
M_PERTURB does work for realloc (which just calls free internally).  I can even verify that it puts my perturb bytes into the free'd memory.  All fine and good.

Unfortunately after installing my perturb byte, something else inside malloc decides to put the value 0 into the first 4 bytes of the free'd memory, overwriting my nice invalid pointer perturb byte ;(

That happens to correspond to transmem_list->next.  When we then try to use the value, it just looks like a null pointer, which the code assumes is the end of the list.  So M_PERTURB isn't particularly helpful in generating a testcase for this bug.
Comment 3 Andreas Jaeger 2012-06-21 19:26:46 UTC
Comment on attachment 6463 [details]
Potential fix

Jeff, the patch looks fine to me. I suggest to send it to libc-alpha to review to see whether Carlos likes to include it or not for 2.16. Thanks!
Comment 4 Rich Felker 2012-06-21 23:34:25 UTC
A worse manifestation of this bug is in multithreaded programs: the just-freed memory could be obtained by another thread and thus the code would not crash, but would instead read memory now belonging to another thread and use it as a pointer. This could have serious consequences, possibly much worse than a crash...
Comment 5 Andreas Jaeger 2012-06-22 08:20:08 UTC
Fixed now, thanks!

commit 006dd86111c44572dbd3b26e9c63dd0f834d7762
Author: Jeff Law <law@redhat.com>
Date:   Thu Jun 21 17:15:38 2012 -0600

            [BZ #14277]
            * intl/dcigettext.c (_nl_find_msg): Avoid use after potential
            free.  Simplify list management for _LIBC case.