Improve buffer-overflow checking (Bug#8873).

This commit is contained in:
Paul Eggert 2011-06-16 14:25:42 -07:00
commit 393d71f34c
6 changed files with 61 additions and 58 deletions

View file

@ -1,3 +1,29 @@
2011-06-16 Paul Eggert <eggert@cs.ucla.edu>
Improve buffer-overflow checking (Bug#8873).
* fileio.c (Finsert_file_contents):
* insdel.c (insert_from_buffer_1, replace_range, replace_range_2):
Remove the old (too-loose) buffer overflow checks.
They weren't needed, since make_gap checks for buffer overflow.
* insdel.c (make_gap_larger): Catch buffer overflows that were missed.
The old code merely checked for Emacs fixnum overflow, and relied
on undefined (wraparound) behavior. The new code avoids undefined
behavior, and also checks for ptrdiff_t and/or size_t overflow.
* editfns.c (Finsert_char): Don't dump core with very negative counts.
Tune. Don't use wider integers than needed. Don't use alloca.
Use a bigger 'string' buffer. Rewrite to avoid 'n > 0' test.
* insdel.c (replace_range): Fix buf overflow when insbytes < outgoing.
* insdel.c, lisp.h (buffer_overflow): New function.
(insert_from_buffer_1, replace_range, replace_range_2):
* insdel.c (make_gap_larger):
* editfns.c (Finsert_char):
* fileio.c (Finsert_file_contents): Use it, to normalize wording.
* buffer.h (BUF_BYTES_MAX): Cast to ptrdiff_t so that it's signed.
2011-06-15 Paul Eggert <eggert@cs.ucla.edu>
Integer overflow and signedness fixes (Bug#8873).

View file

@ -309,8 +309,10 @@ while (0)
/* Maximum number of bytes in a buffer.
A buffer cannot contain more bytes than a 1-origin fixnum can represent,
nor can it be so large that C pointer arithmetic stops working. */
#define BUF_BYTES_MAX min (MOST_POSITIVE_FIXNUM - 1, min (SIZE_MAX, PTRDIFF_MAX))
nor can it be so large that C pointer arithmetic stops working.
The ptrdiff_t cast ensures that this is signed, not unsigned. */
#define BUF_BYTES_MAX \
(ptrdiff_t) min (MOST_POSITIVE_FIXNUM - 1, min (SIZE_MAX, PTRDIFF_MAX))
/* Return the address of byte position N in current buffer. */

View file

@ -2328,12 +2328,11 @@ The optional third arg INHERIT, if non-nil, says to inherit text properties
from adjoining text, if those properties are sticky. */)
(Lisp_Object character, Lisp_Object count, Lisp_Object inherit)
{
register char *string;
register EMACS_INT stringlen;
register int i;
int i, stringlen;
register EMACS_INT n;
int c, len;
unsigned char str[MAX_MULTIBYTE_LENGTH];
char string[4000];
CHECK_CHARACTER (character);
CHECK_NUMBER (count);
@ -2343,16 +2342,15 @@ from adjoining text, if those properties are sticky. */)
len = CHAR_STRING (c, str);
else
str[0] = c, len = 1;
if (BUF_BYTES_MAX / len < XINT (count))
error ("Maximum buffer size would be exceeded");
n = XINT (count) * len;
if (n <= 0)
if (XINT (count) <= 0)
return Qnil;
stringlen = min (n, 256 * len);
string = (char *) alloca (stringlen);
if (BUF_BYTES_MAX / len < XINT (count))
buffer_overflow ();
n = XINT (count) * len;
stringlen = min (n, sizeof string - sizeof string % len);
for (i = 0; i < stringlen; i++)
string[i] = str[i % len];
while (n >= stringlen)
while (n > stringlen)
{
QUIT;
if (!NILP (inherit))
@ -2361,13 +2359,10 @@ from adjoining text, if those properties are sticky. */)
insert (string, stringlen);
n -= stringlen;
}
if (n > 0)
{
if (!NILP (inherit))
insert_and_inherit (string, n);
else
insert (string, n);
}
if (!NILP (inherit))
insert_and_inherit (string, n);
else
insert (string, n);
return Qnil;
}

View file

@ -3264,7 +3264,7 @@ variable `last-coding-system-used' to the coding system actually used. */)
platform that allows file sizes greater than the maximum off_t value. */
if (! not_regular
&& ! (0 <= st.st_size && st.st_size <= BUF_BYTES_MAX))
error ("Maximum buffer size exceeded");
buffer_overflow ();
/* Prevent redisplay optimizations. */
current_buffer->clip_changed = 1;
@ -3800,16 +3800,7 @@ variable `last-coding-system-used' to the coding system actually used. */)
}
if (! not_regular)
{
register Lisp_Object temp;
total = XINT (end) - XINT (beg);
/* Make sure point-max won't overflow after this insertion. */
XSETINT (temp, total);
if (total != XINT (temp))
error ("Maximum buffer size exceeded");
}
total = XINT (end) - XINT (beg);
else
/* For a special file, all we can do is guess. */
total = READ_BUF_SIZE;

View file

@ -391,6 +391,12 @@ adjust_markers_for_replace (EMACS_INT from, EMACS_INT from_byte,
}
void
buffer_overflow (void)
{
error ("Maximum buffer size exceeded");
}
/* Make the gap NBYTES_ADDED bytes longer. */
static void
@ -400,16 +406,16 @@ make_gap_larger (EMACS_INT nbytes_added)
EMACS_INT real_gap_loc;
EMACS_INT real_gap_loc_byte;
EMACS_INT old_gap_size;
EMACS_INT current_size = Z_BYTE - BEG_BYTE + GAP_SIZE;
enum { enough_for_a_while = 2000 };
/* If we have to get more space, get enough to last a while. */
nbytes_added += 2000;
if (BUF_BYTES_MAX - current_size < nbytes_added)
buffer_overflow ();
{ EMACS_INT total_size = Z_BYTE - BEG_BYTE + GAP_SIZE + nbytes_added;
if (total_size < 0
/* Don't allow a buffer size that won't fit in a Lisp integer. */
|| total_size != XINT (make_number (total_size)))
error ("Buffer exceeds maximum size");
}
/* If we have to get more space, get enough to last a while;
but do not exceed the maximum buffer size. */
nbytes_added = min (nbytes_added + enough_for_a_while,
BUF_BYTES_MAX - current_size);
enlarge_buffer_text (current_buffer, nbytes_added);
@ -1063,7 +1069,6 @@ static void
insert_from_buffer_1 (struct buffer *buf,
EMACS_INT from, EMACS_INT nchars, int inherit)
{
register Lisp_Object temp;
EMACS_INT chunk, chunk_expanded;
EMACS_INT from_byte = buf_charpos_to_bytepos (buf, from);
EMACS_INT to_byte = buf_charpos_to_bytepos (buf, from + nchars);
@ -1102,11 +1107,6 @@ insert_from_buffer_1 (struct buffer *buf,
outgoing_nbytes = outgoing_before_gap + outgoing_after_gap;
}
/* Make sure point-max won't overflow after this insertion. */
XSETINT (temp, outgoing_nbytes + Z);
if (outgoing_nbytes + Z != XINT (temp))
error ("Maximum buffer size exceeded");
/* Do this before moving and increasing the gap,
because the before-change hooks might move the gap
or make it smaller. */
@ -1303,7 +1303,6 @@ replace_range (EMACS_INT from, EMACS_INT to, Lisp_Object new,
EMACS_INT insbytes = SBYTES (new);
EMACS_INT from_byte, to_byte;
EMACS_INT nbytes_del, nchars_del;
register Lisp_Object temp;
struct gcpro gcpro1;
INTERVAL intervals;
EMACS_INT outgoing_insbytes = insbytes;
@ -1347,11 +1346,6 @@ replace_range (EMACS_INT from, EMACS_INT to, Lisp_Object new,
outgoing_insbytes
= count_size_as_multibyte (SDATA (new), insbytes);
/* Make sure point-max won't overflow after this insertion. */
XSETINT (temp, Z_BYTE - nbytes_del + insbytes);
if (Z_BYTE - nbytes_del + insbytes != XINT (temp))
error ("Maximum buffer size exceeded");
GCPRO1 (new);
/* Make sure the gap is somewhere in or next to what we are deleting. */
@ -1383,8 +1377,8 @@ replace_range (EMACS_INT from, EMACS_INT to, Lisp_Object new,
if (Z - GPT < END_UNCHANGED)
END_UNCHANGED = Z - GPT;
if (GAP_SIZE < insbytes)
make_gap (insbytes - GAP_SIZE);
if (GAP_SIZE < outgoing_insbytes)
make_gap (outgoing_insbytes - GAP_SIZE);
/* Copy the string text into the buffer, perhaps converting
between single-byte and multibyte. */
@ -1482,7 +1476,6 @@ replace_range_2 (EMACS_INT from, EMACS_INT from_byte,
int markers)
{
EMACS_INT nbytes_del, nchars_del;
Lisp_Object temp;
CHECK_MARKERS ();
@ -1492,11 +1485,6 @@ replace_range_2 (EMACS_INT from, EMACS_INT from_byte,
if (nbytes_del <= 0 && insbytes == 0)
return;
/* Make sure point-max won't overflow after this insertion. */
XSETINT (temp, Z_BYTE - nbytes_del + insbytes);
if (Z_BYTE - nbytes_del + insbytes != XINT (temp))
error ("Maximum buffer size exceeded");
/* Make sure the gap is somewhere in or next to what we are deleting. */
if (from > GPT)
gap_right (from, from_byte);

View file

@ -2635,6 +2635,7 @@ extern void init_image (void);
extern Lisp_Object Qinhibit_modification_hooks;
extern void move_gap (EMACS_INT);
extern void move_gap_both (EMACS_INT, EMACS_INT);
extern void buffer_overflow (void) NO_RETURN;
extern void make_gap (EMACS_INT);
extern EMACS_INT copy_text (const unsigned char *, unsigned char *,
EMACS_INT, int, int);