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

Partially revert commits 2b766585f9 and
de2fd463b1, 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.
This commit is contained in:
Eric Biggers 2013-10-11 22:29:38 +05:30 committed by Siddhesh Poyarekar
parent 75b4202ab0
commit 3d110c7c6e
8 changed files with 50 additions and 36 deletions

View File

@ -1,3 +1,20 @@
2013-10-11 Eric Biggers <ebiggers3@gmail.com>
[BZ #15362]
* libio/fileops.c (_IO_new_file_write): Return count of bytes
written.
(_IO_new_file_xsputn): Don't return EOF if nothing has been
written.
* libio/iofwrite.c (_IO_fwrite): Return count if bytes were
written to buffer but not flushed.
* libio/iofwrite_u.c: Likewise.
* libio/iopadn.c: Return bytes returned even if EOF was
encountered.
* libio/iowpadn.c: Likewise.
* stdio-common/vfprintf.c [COMPILE_WPRINTF] (PAD): Return error
if _IO_padn does not write the whole buffer.
[!COMPILE_WPRINTF] (PAD): Likewise.
2013-10-10 David S. Miller <davem@davemloft.net>
* sysdeps/posix/dirstream.h (struct __dirstream): Fix alignment of

12
NEWS
View File

@ -9,12 +9,12 @@ Version 2.19
* The following bugs are resolved with this release:
156, 431, 13982, 13985, 14155, 14547, 14699, 15048, 15400, 15427, 15522,
15531, 15532, 15608, 15609, 15610, 15632, 15640, 15680, 15681, 15723,
15734, 15735, 15736, 15748, 15749, 15754, 15760, 15797, 15844, 15849,
15855, 15856, 15857, 15859, 15867, 15886, 15887, 15890, 15892, 15893,
15895, 15897, 15905, 15909, 15919, 15921, 15923, 15939, 15963, 15966,
15988, 16034.
156, 431, 13982, 13985, 14155, 14547, 14699, 15048, 15362, 15400, 15427,
15522, 15531, 15532, 15608, 15609, 15610, 15632, 15640, 15680, 15681,
15723, 15734, 15735, 15736, 15748, 15749, 15754, 15760, 15797, 15844,
15849, 15855, 15856, 15857, 15859, 15867, 15886, 15887, 15890, 15892,
15893, 15895, 15897, 15905, 15909, 15919, 15921, 15923, 15939, 15963,
15966, 15988, 16034.
* CVE-2012-4412 The strcoll implementation caches indices and rules for
large collation sequences to optimize multiple passes. This cache

View File

@ -1245,10 +1245,9 @@ _IO_new_file_write (f, data, n)
_IO_ssize_t n;
{
_IO_ssize_t to_do = n;
_IO_ssize_t count = 0;
while (to_do > 0)
{
count = (__builtin_expect (f->_flags2
_IO_ssize_t count = (__builtin_expect (f->_flags2
& _IO_FLAGS2_NOTCANCEL, 0)
? write_not_cancel (f->_fileno, data, to_do)
: write (f->_fileno, data, to_do));
@ -1263,7 +1262,7 @@ _IO_new_file_write (f, data, n)
n -= to_do;
if (f->_offset >= 0)
f->_offset += n;
return count < 0 ? count : n;
return n;
}
_IO_size_t
@ -1323,13 +1322,11 @@ _IO_new_file_xsputn (f, data, n)
_IO_size_t block_size, do_write;
/* Next flush the (full) buffer. */
if (_IO_OVERFLOW (f, EOF) == EOF)
/* If nothing else has to be written or nothing has been written, we
must not signal the caller that the call was even partially
successful. */
return (to_do == 0 || to_do == n) ? EOF : n - to_do;
/* If nothing else has to be written we must not signal the
caller that everything has been written. */
return to_do == 0 ? EOF : n - to_do;
/* Try to maintain alignment: write a whole number of blocks.
dont_write is what gets left over. */
/* Try to maintain alignment: write a whole number of blocks. */
block_size = f->_IO_buf_end - f->_IO_buf_base;
do_write = to_do - (block_size >= 128 ? to_do % block_size : 0);

View File

@ -42,12 +42,12 @@ _IO_fwrite (buf, size, count, fp)
if (_IO_vtable_offset (fp) != 0 || _IO_fwide (fp, -1) == -1)
written = _IO_sputn (fp, (const char *) buf, request);
_IO_release_lock (fp);
/* We are guaranteed to have written all of the input, none of it, or
some of it. */
if (written == request)
/* We have written all of the input in case the return value indicates
this or EOF is returned. The latter is a special case where we
simply did not manage to flush the buffer. But the data is in the
buffer and therefore written as far as fwrite is concerned. */
if (written == request || written == EOF)
return count;
else if (written == EOF)
return 0;
else
return written / size;
}

View File

@ -44,12 +44,12 @@ fwrite_unlocked (buf, size, count, fp)
if (_IO_fwide (fp, -1) == -1)
{
written = _IO_sputn (fp, (const char *) buf, request);
/* We are guaranteed to have written all of the input, none of it, or
some of it. */
if (written == request)
/* We have written all of the input in case the return value indicates
this or EOF is returned. The latter is a special case where we
simply did not manage to flush the buffer. But the data is in the
buffer and therefore written as far as fwrite is concerned. */
if (written == request || written == EOF)
return count;
else if (written == EOF)
return 0;
}
return written / size;

View File

@ -59,7 +59,7 @@ _IO_padn (fp, pad, count)
w = _IO_sputn (fp, padptr, PADSIZE);
written += w;
if (w != PADSIZE)
return w == EOF ? w : written;
return written;
}
if (i > 0)

View File

@ -65,7 +65,7 @@ _IO_wpadn (fp, pad, count)
w = _IO_sputn (fp, (char *) padptr, PADSIZE);
written += w;
if (w != PADSIZE)
return w == EOF ? w : written;
return written;
}
if (i > 0)

View File

@ -90,13 +90,13 @@
do { \
if (width > 0) \
{ \
unsigned int d = _IO_padn (s, (Padchar), width); \
if (__glibc_unlikely (d == EOF)) \
_IO_ssize_t written = _IO_padn (s, (Padchar), width); \
if (__glibc_unlikely (written != width)) \
{ \
done = -1; \
goto all_done; \
} \
done_add (d); \
done_add (written); \
} \
} while (0)
# define PUTC(C, F) _IO_putc_unlocked (C, F)
@ -119,13 +119,13 @@
do { \
if (width > 0) \
{ \
unsigned int d = _IO_wpadn (s, (Padchar), width); \
if (__glibc_unlikely (d == EOF)) \
_IO_ssize_t written = _IO_wpadn (s, (Padchar), width); \
if (__glibc_unlikely (written != width)) \
{ \
done = -1; \
goto all_done; \
} \
done_add (d); \
done_add (written); \
} \
} while (0)
# define PUTC(C, F) _IO_putwc_unlocked (C, F)