two undefined behaviour occurrences in m4

Bruno Haible <[email protected]> Sat, 02 Dec 2023 06:41:17 +0100
Newsgroups gmane.comp.gnu.m4.bugs
Message-ID <18274337.aqqpvuueIm@nimes>
Hi,

After fixing the Gnulib bug reported at
  <https://lists.gnu.org/archive/html/bug-m4/2023-02/msg00000.html>
I wanted to check whether the newest m4 snapshot passes its tests
with
  CC="clang -fsanitize=address,undefined,signed-integer-overflow,shift,integer-divide-by-zero -fsanitize-address-use-after-scope -fno-sanitize-recover=all"
And it doesn't. There are two problems:

1) In the "make check" run (attached: make-check-1.log), there are many
occurrences of
   path.c:72:23: runtime error: applying non-zero offset 1 to null pointer

This code
  ----------------------------------------
      path_end = strchr (path, ':');
      if (path_end)
        *path_end = '\0';
      add_include_directory (path);
      path = path_end + 1;
  ----------------------------------------
computes a NULL pointer + 1. Which is invalid according to ISO C 23 § 6.5.6.(9)
  "... If the pointer operand and the result do not point to elements
   of the same array object or one past the last element of the array object,
   the behavior is undefined..." 

2) After fixing this, in the next "make check" run (attached: make-check-2.log),
there are many occurrences of
  macro.c:388:3: runtime error: addition of unsigned offset to 0x521000008d28 overflowed to 0x521000008d10

The problem here is that the code is adding a pointer value such as
0x521000008d28 with an unsigned offset of 0xffffffffffffffe8, and
this sum overflows. It is invalid according to ISO C 23 § 6.5.6.(9)
  "If the addition or subtraction produces an overflow, the behavior
   is undefined."

Find attached a patch that fixes both issues.

OK to push it?
make-check-1.log (text/x-log, 209.5 KB) - not displayed
make-check-2.log (text/x-log, 212.8 KB) - not displayed
0001-Fix-two-occurrences-of-undefined-behaviour.patch (text/x-patch, 1.5 KB)
From 7f5cabf73de36c8d9a8eb60040db116f3fd1dcb5 Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email protected]>
Date: Sat, 2 Dec 2023 06:38:14 +0100
Subject: [PATCH] Fix two occurrences of undefined behaviour.

* src/path.c (include_env_init): When path_end becomes NULL, terminate
the loop without computing path_end + 1.
* src/macro.c (expand_macro): Pass a signed negative value to
obstack_blank_fast. This avoids a pointer overflow.
---
 src/macro.c | 2 +-
 src/path.c  | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/macro.c b/src/macro.c
index 6052cec4..15167d68 100644
--- a/src/macro.c
+++ b/src/macro.c
@@ -385,5 +385,5 @@ expand_macro (symbol *sym)
     obstack_free (&argc_stack, argv[0]);
   else
     obstack_free (&arguments, NULL);
-  obstack_blank_fast (&argv_stack, -argc * sizeof (token_data *));
+  obstack_blank_fast (&argv_stack, - argc * (ptrdiff_t) sizeof (token_data *));
 }
diff --git a/src/path.c b/src/path.c
index bba197cd..09ecf5e9 100644
--- a/src/path.c
+++ b/src/path.c
@@ -50,7 +50,6 @@ void
 include_env_init (void)
 {
   char *path;
-  char *path_end;
   char *env_path;
 
   if (no_gnu_extensions)
@@ -63,15 +62,16 @@ include_env_init (void)
   env_path = xstrdup (env_path);
   path = env_path;
 
-  do
+  for (;;)
     {
-      path_end = strchr (path, ':');
+      char *path_end = strchr (path, ':');
       if (path_end)
         *path_end = '\0';
       add_include_directory (path);
+      if (!path_end)
+        break;
       path = path_end + 1;
     }
-  while (path_end);
   free (env_path);
 }
 
-- 
2.34.1