Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/indent indent: fix undefined behavior in buffer hand...



details:   https://anonhg.NetBSD.org/src/rev/63267874a8d5
branches:  trunk
changeset: 1024598:63267874a8d5
user:      rillig <rillig%NetBSD.org@localhost>
date:      Fri Oct 29 19:12:48 2021 +0000

description:
indent: fix undefined behavior in buffer handling

Adding an arbitrary integer to a pointer may result in an out of bounds
pointer, so replace the addition with a pointer subtraction.

In the buffer handling functions, handle 'buf' and 'l' before 's' and
'e', since they are pairs.

In inbuf_read_line, use 's' instead of 'buf' to make the code easier to
understand for human readers.

No functional change.

diffstat:

 usr.bin/indent/indent.c     |  29 ++++++++++++++++-------------
 usr.bin/indent/io.c         |   8 ++++----
 usr.bin/indent/pr_comment.c |  13 ++++++++-----
 3 files changed, 28 insertions(+), 22 deletions(-)

diffs (157 lines):

diff -r 10adf1239a1d -r 63267874a8d5 usr.bin/indent/indent.c
--- a/usr.bin/indent/indent.c   Fri Oct 29 19:12:29 2021 +0000
+++ b/usr.bin/indent/indent.c   Fri Oct 29 19:12:48 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: indent.c,v 1.172 2021/10/29 18:50:52 rillig Exp $      */
+/*     $NetBSD: indent.c,v 1.173 2021/10/29 19:12:48 rillig Exp $      */
 
 /*-
  * SPDX-License-Identifier: BSD-4-Clause
@@ -43,7 +43,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__NetBSD__)
-__RCSID("$NetBSD: indent.c,v 1.172 2021/10/29 18:50:52 rillig Exp $");
+__RCSID("$NetBSD: indent.c,v 1.173 2021/10/29 19:12:48 rillig Exp $");
 #elif defined(__FreeBSD__)
 __FBSDID("$FreeBSD: head/usr.bin/indent/indent.c 340138 2018-11-04 19:24:49Z oshogbo $");
 #endif
@@ -390,11 +390,11 @@
 {
     size_t size = 200;
     buf->buf = xmalloc(size);
-    buf->buf[0] = ' ';         /* allow accessing buf->e[-1] */
+    buf->l = buf->buf + size - 5 /* safety margin */;
+    buf->s = buf->buf + 1;     /* allow accessing buf->e[-1] */
+    buf->e = buf->s;
+    buf->buf[0] = ' ';
     buf->buf[1] = '\0';
-    buf->s = buf->buf + 1;
-    buf->e = buf->s;
-    buf->l = buf->buf + size - 5;      /* safety margin */
 }
 
 static size_t
@@ -404,20 +404,21 @@
 }
 
 void
-buf_expand(struct buffer *buf, size_t desired_size)
+buf_expand(struct buffer *buf, size_t add_size)
 {
-    size_t nsize = (size_t)(buf->l - buf->s) + 400 + desired_size;
+    size_t new_size = (size_t)(buf->l - buf->s) + 400 + add_size;
     size_t len = buf_len(buf);
-    buf->buf = xrealloc(buf->buf, nsize);
-    buf->e = buf->buf + len + 1;
-    buf->l = buf->buf + nsize - 5;
+    buf->buf = xrealloc(buf->buf, new_size);
+    buf->l = buf->buf + new_size - 5;
     buf->s = buf->buf + 1;
+    buf->e = buf->s + len;
+    /* At this point, the buffer may not be null-terminated anymore. */
 }
 
 static void
 buf_reserve(struct buffer *buf, size_t n)
 {
-    if (buf->e + n >= buf->l)
+    if (n >= (size_t)(buf->l - buf->e))
        buf_expand(buf, n);
 }
 
@@ -467,7 +468,9 @@
 
     inp.buf = xmalloc(10);
     inp.l = inp.buf + 8;
-    inp.s = inp.e = inp.buf;
+    inp.s = inp.buf;
+    inp.e = inp.buf;
+
     line_no = 1;
     had_eof = ps.in_decl = ps.decl_on_line = break_comma = false;
 
diff -r 10adf1239a1d -r 63267874a8d5 usr.bin/indent/io.c
--- a/usr.bin/indent/io.c       Fri Oct 29 19:12:29 2021 +0000
+++ b/usr.bin/indent/io.c       Fri Oct 29 19:12:48 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: io.c,v 1.105 2021/10/29 18:18:03 rillig Exp $  */
+/*     $NetBSD: io.c,v 1.106 2021/10/29 19:12:48 rillig Exp $  */
 
 /*-
  * SPDX-License-Identifier: BSD-4-Clause
@@ -43,7 +43,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__NetBSD__)
-__RCSID("$NetBSD: io.c,v 1.105 2021/10/29 18:18:03 rillig Exp $");
+__RCSID("$NetBSD: io.c,v 1.106 2021/10/29 19:12:48 rillig Exp $");
 #elif defined(__FreeBSD__)
 __FBSDID("$FreeBSD: head/usr.bin/indent/io.c 334927 2018-06-10 16:44:18Z pstef $");
 #endif
@@ -448,8 +448,8 @@
     inp.s = inp.buf;
     inp.e = p;
 
-    if (p - inp.buf >= 3 && p[-3] == '*' && p[-2] == '/') {
-       if (strncmp(inp.buf, "/**INDENT**", 11) == 0)
+    if (p - inp.s >= 3 && p[-3] == '*' && p[-2] == '/') {
+       if (strncmp(inp.s, "/**INDENT**", 11) == 0)
            inbuf_read_line();  /* flush indent error message */
        else
            parse_indent_comment();
diff -r 10adf1239a1d -r 63267874a8d5 usr.bin/indent/pr_comment.c
--- a/usr.bin/indent/pr_comment.c       Fri Oct 29 19:12:29 2021 +0000
+++ b/usr.bin/indent/pr_comment.c       Fri Oct 29 19:12:48 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pr_comment.c,v 1.88 2021/10/29 17:50:37 rillig Exp $   */
+/*     $NetBSD: pr_comment.c,v 1.89 2021/10/29 19:12:48 rillig Exp $   */
 
 /*-
  * SPDX-License-Identifier: BSD-4-Clause
@@ -43,7 +43,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__NetBSD__)
-__RCSID("$NetBSD: pr_comment.c,v 1.88 2021/10/29 17:50:37 rillig Exp $");
+__RCSID("$NetBSD: pr_comment.c,v 1.89 2021/10/29 19:12:48 rillig Exp $");
 #elif defined(__FreeBSD__)
 __FBSDID("$FreeBSD: head/usr.bin/indent/pr_comment.c 334927 2018-06-10 16:44:18Z pstef $");
 #endif
@@ -58,7 +58,7 @@
 static void
 com_add_char(char ch)
 {
-    if (com.e + 1 >= com.l)
+    if (1 >= com.l - com.e)
        buf_expand(&com, 1);
     *com.e++ = ch;
 }
@@ -69,7 +69,7 @@
     if (!opt.star_comment_cont)
        return;
     size_t len = 3;
-    if (com.e + len >= com.l)
+    if (len >= (size_t)(com.l - com.e))
        buf_expand(&com, len);
     memcpy(com.e, " * ", len);
     com.e += len;
@@ -78,7 +78,7 @@
 static void
 com_terminate(void)
 {
-    if (com.e + 1 >= com.l)
+    if (1 >= com.l - com.e)
        buf_expand(&com, 1);
     *com.e = '\0';
 }
@@ -190,6 +190,9 @@
        /*
         * XXX: ordered comparison between pointers from different objects
         * invokes undefined behavior (C99 6.5.8).
+        *
+        * XXX: It's easier to understand if inp.s is used instead of inp.buf,
+        * since inp.buf is only intended to be used for allocation purposes.
         */
        start = inp.s >= save_com && inp.s < save_com + sc_size ?
            sc_buf : inp.buf;



Home | Main Index | Thread Index | Old Index