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

["ebiggers3 at gmail dot com" <sourceware-bugzilla@sourceware.org>]

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.