[Bug stdio/15362] New: fwrite() may read beyond end of specified buffer

[Bug stdio/15362] New: fwrite() may read beyond end of specified buffer

[ebiggers3 at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15362

             Bug #: 15362
           Summary: fwrite() may read beyond end of specified buffer
           Product: glibc
           Version: 2.17
            Status: NEW
          Severity: critical
          Priority: P2
         Component: stdio
        AssignedTo: unassigned@sourceware.org
        ReportedBy: ebiggers3@gmail.com
    Classification: Unclassified


Created attachment 6979
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6979
Program to reproduce bug (only when write fails), and gdb backtrace

I have found a bug in glibc's handling of errors in the write() system call,
present in the latest version in git, that I was able to bisect down to the
following commit:

    commit 2b766585f9b4ffabeef2f36200c275976b93f2c7
    Author: Siddhesh Poyarekar <siddhesh@redhat.com>
    Date:   Fri Nov 16 19:13:11 2012 +0530

        printf should return negative value on error

        [BZ #11741]
        Fixed bug where printf and family may return a spurious success when
        printing padded formats.

The bug can cause fwrite(), and possibly other functions, to read past the
buffer it was passed.  It may also be possible that data directly following the
passed buffer could be written to the file.  For both of these reasons I marked
this bug as critical.

I initially noticed this bug when fwrite() accessed memory after the buffer it
was supposed to access when writing to a full filesystem.  This was tested with
valgrind and also confirmed by allocating the buffer with mmap() and setting
PROT_NONE on the following page with mprotect().

The following describes the cause of the bug as best as I could determine from
running the program in gdb and reading the glibc code:

1. fwrite() is called, which calls _IO_new_file_xsputn(), then new_do_write(),
then _IO_new_file_write() in libio/fileops.c.

2. On error, _IO_new_file_write() returns -1.  This behavior was introduced by
commit 2b766585f9b4ffabeef2f36200c275976b93f2c7.  The previous behavior was to
return a short count (number of bytes written).

3. new_do_write() in libio/fileops.c returns the -1 'count' it got from
_IO_new_file_write().

4. _IO_new_file_xsputn() subtracts 'count' from 'to_do', but 'count' is
(size_t)-1, causing 'to_do' to be incremented by 1.

5. Still in _IO_new_file_xsputn():  'count', which is (size_t)-1, is not less
than 'do_write', so execution continues to _IO_default_xsputn().  But the
buffer
passed to _IO_default_xsputn() is incremented by 'do_write' bytes, even though
these bytes probably were NOT written.  The buffer may now *start* at the *end*
of the data that was supposed to be written, yet the length parameter may be
up to 1 more than the original size passed into _IO_new_file_xsputn().

6.  In _IO_default_xsputn(): __mempcpy() attempts to fill the _IO_FILE buffer
with data from the input buffer.  This data may be past the end of the allowed
buffer, causing a memory access violation; alternatively, the memory copy could
succeed, which may cause uninitialized memory, or even memory containing
sensitive information, to be passed to the underlying write() system call.

How to visibly reproduce the bug:  A test program 'test.c' is attached.  This
program simply opens a file in the current directory and writes 32768 bytes to
it.  The 32768 bytes are taken from a memory mapping (32768 + 4096) bytes in
length, the last page of which is set to PROT_NONE to easily see the access
violation.  So the actual buffer passed to fwrite() is valid for exactly the
provided length but no more.  To easily cause write() to fail: on Linux, create
an empty directory 'tmp', then mount tmpfs on it with a small size limit ('sudo
mount tmpfs -t tmpfs tmp -o size=20K').  Then run the test program in this
directory, which will segfault, contrary to the expected behavior.  Backtrace
of
this is attached.

I did not attach a patch because I am not otherwise highly familiar with the
glibc code and am not sure I would be able to correctly fix both this problem
and the problem that the problematic commit was trying to fix, but I could try
to write one if no one else does.  Also given the code path described above, I
believe this is purely a glibc bug and does not rely on any particular
compiler,
kernel version, or architecture.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug stdio/15362] fwrite() may read beyond end of specified buffer

[ebiggers3 at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15362

Eric Biggers <ebiggers3 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebiggers3 at gmail dot com

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug stdio/15362] fwrite() may read beyond end of specified buffer

[aj at suse dot de]

http://sourceware.org/bugzilla/show_bug.cgi?id=15362

Andreas Jaeger <aj at suse dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aj at suse dot de, siddhesh
                   |                            |at redhat dot com

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug stdio/15362] fwrite() may read beyond end of specified buffer

[siddhesh at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15362

Siddhesh Poyarekar <siddhesh at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
         AssignedTo|siddhesh at redhat dot com  |unassigned at sourceware
                   |                            |dot org

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug stdio/15362] fwrite() may read beyond end of specified buffer

[licquia at linuxfoundation dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=15362

Jeff Licquia <licquia at linuxfoundation dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |licquia at linuxfoundation dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug stdio/15362] fwrite() may read beyond end of specified buffer

[norman.shulman@n-dimension.com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15362

Norman Shulman <norman.shulman@n-dimension.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |norman.shulman@n-dimension.
                   |                            |com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug stdio/15362] fwrite() may read beyond end of specified buffer

[siddhesh at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15362

Siddhesh Poyarekar <siddhesh at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #2 from Siddhesh Poyarekar <siddhesh at redhat dot com> ---
Fixed in master:

commit 3d110c7c6e6549bd4124fce49cdc672f9e449799
Author: Eric Biggers <ebiggers3@gmail.com>
Date:   Fri Oct 11 22:29:38 2013 +0530

    Fix fwrite() reading beyond end of buffer in error path

    Partially revert commits 2b766585f9b4ffabeef2f36200c275976b93f2c7 and
    de2fd463b1c0310d75084b6d774fb974075a4ad9, which were intended to fix
BZ#11741
    but caused another, likely worse bug, namely that fwrite() and fputs()
could,
    in an error path, read data beyond the end of the specified buffer, and
    potentially even write this data to the file.

    Fix BZ#11741 properly by checking the return value from _IO_padn() in
    stdio-common/vfprintf.c.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug stdio/15362] fwrite() may read beyond end of specified buffer

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15362

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com
              Flags|                            |security+

--- Comment #3 from Florian Weimer <fweimer at redhat dot com> ---
This appears to be a potential information disclosure issue.

-- 
You are receiving this mail because:
You are on the CC list for the bug.