FYI: branch-1_4 character traits

[Eric Blake-1]

Compiling on Solaris identified this potential security hole on
the branch.

if gcc -DHAVE_CONFIG_H -I. -I. -I..  -I ../lib   -Wall -Werror -O2 -MT
macro.o -MD -MP -MF ".deps/macro.Tpo" -c -o macro.o macro.c; \
then mv -f ".deps/macro.Tpo" ".deps/macro.Po"; else rm -f ".deps/macro.Tpo";
exit 1; fi
macro.c: In function `expand_argument':
macro.c:126: warning: subscript has type `char'
make[1]: *** [macro.o] Error 1

In short, POSIX states that the character trait macros are
undefined for 8-bit signed char, because system headers
are allowed to use the char as an array index, and an
out-of-bounds reference might happen.  to_uchar is
borrowed from coreutils to circumvent this.

2006-06-15  Eric Blake  <address@hidden>

        * src/m4.h (to_uchar): New function.
        * src/eval.c (eval_lex): Use it to avoid passing signed char to
        isdigit, isalpha, isupper, islower, isspace.
        * src/builtin.c (expand_user_macro): Likewise.
        * src/format.c (format): Likewise.
        * src/macro.c (expand_argument): Likewise.
        * NEWS: Document this security fix.

Index: NEWS
===================================================================
RCS file: /sources/m4/m4/NEWS,v
retrieving revision 1.1.1.1.2.18
diff -u -r1.1.1.1.2.18 NEWS
--- NEWS        15 Jun 2006 13:08:45 -0000      1.1.1.1.2.18
+++ NEWS        15 Jun 2006 21:44:37 -0000
@@ -22,6 +22,7 @@
   the sequence "define(`f',`1')f(define(`f',`2'))f" is now documented to
   result in "12", rather than the previously undocumented "22".
 * Update the regex engine to fix several bugs.
+* Fix a potential crash on machines where char is signed.
 
 Version 1.4.4 - October 2005, by Gary V. Vaughan
 
Index: src/builtin.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/builtin.c,v
retrieving revision 1.1.1.1.2.10
diff -u -r1.1.1.1.2.10 builtin.c
--- src/builtin.c       15 Jun 2006 21:29:16 -0000      1.1.1.1.2.10
+++ src/builtin.c       15 Jun 2006 21:44:37 -0000
@@ -1716,7 +1716,7 @@
            }
          else
            {
-             for (i = 0; isdigit (*text); text++)
+             for (i = 0; isdigit (to_uchar (*text)); text++)
                i = i*10 + (*text - '0');
            }
          if (i < argc)
Index: src/eval.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/eval.c,v
retrieving revision 1.1.1.1.2.1
diff -u -r1.1.1.1.2.1 eval.c
--- src/eval.c  1 May 2005 11:54:12 -0000       1.1.1.1.2.1
+++ src/eval.c  15 Jun 2006 21:44:37 -0000
@@ -1,6 +1,6 @@
 /* GNU m4 -- A simple macro processor
 
-   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994
+   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2006
    Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
@@ -101,7 +101,7 @@
 static eval_token
 eval_lex (eval_t *val)
 {
-  while (isspace (*eval_text))
+  while (isspace (to_uchar (*eval_text)))
     eval_text++;
 
   last_text = eval_text;
@@ -109,7 +109,7 @@
   if (*eval_text == '\0')
     return EOTEXT;
 
-  if (isdigit (*eval_text))
+  if (isdigit (to_uchar (*eval_text)))
     {
       int base, digit;
 
@@ -134,7 +134,7 @@
            case 'R':
              base = 0;
              eval_text++;
-             while (isdigit (*eval_text) && base <= 36)
+             while (isdigit (to_uchar (*eval_text)) && base <= 36)
                base = 10 * base + *eval_text++ - '0';
              if (base == 0 || base > 36 || *eval_text != ':')
                return ERROR;
@@ -151,11 +151,11 @@
       (*val) = 0;
       for (; *eval_text; eval_text++)
        {
-         if (isdigit (*eval_text))
+          if (isdigit (to_uchar (*eval_text)))
            digit = *eval_text - '0';
-         else if (islower (*eval_text))
+         else if (islower (to_uchar (*eval_text)))
            digit = *eval_text - 'a' + 10;
-         else if (isupper (*eval_text))
+         else if (isupper (to_uchar (*eval_text)))
            digit = *eval_text - 'A' + 10;
          else
            break;
Index: src/format.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/format.c,v
retrieving revision 1.1.1.1.2.1
diff -u -r1.1.1.1.2.1 format.c
--- src/format.c        1 May 2005 11:54:12 -0000       1.1.1.1.2.1
+++ src/format.c        15 Jun 2006 21:44:37 -0000
@@ -1,6 +1,6 @@
 /* GNU m4 -- A simple macro processor
 
-   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994
+   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2006
    Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
@@ -225,14 +225,14 @@
          width = ARG_INT (argc, argv);
          fmt++;
        }
-      else if (isdigit (*fmt))
+      else if (isdigit (to_uchar (*fmt)))
        {
          width = 0;
          do
            {
              width = width * 10 + *fmt++ - '0';
            }
-         while (isdigit (*fmt));
+         while (isdigit (to_uchar (*fmt)));
        }
 
       /* Maximum precision.  */
@@ -244,14 +244,14 @@
              prec = ARG_INT (argc, argv);
              ++fmt;
            }
-         else if (isdigit (*fmt))
+         else if (isdigit (to_uchar (*fmt)))
            {
              prec = 0;
              do
                {
                  prec = prec * 10 + *fmt++ - '0';
                }
-             while (isdigit (*fmt))
+             while (isdigit (to_uchar (*fmt)))
                ;
            }
        }
@@ -582,13 +582,13 @@
          width = ARG_INT (argc, argv);
          fmt++;
        }
-      else if (isdigit (*fmt))
+      else if (isdigit (to_uchar (*fmt)))
        {
          do
            {
              fmt++;
            }
-         while (isdigit (*fmt));
+         while (isdigit (to_uchar (*fmt)));
        }
 
       /* Maximum precision.  */
@@ -600,13 +600,13 @@
              prec = ARG_INT (argc, argv);
              ++fmt;
            }
-         else if (isdigit (*fmt))
+         else if (isdigit (to_uchar (*fmt)))
            {
              do
                {
                  fmt++;
                }
-             while (isdigit (*fmt));
+             while (isdigit (to_uchar (*fmt)));
            }
        }
 
Index: src/m4.h
===================================================================
RCS file: /sources/m4/m4/src/m4.h,v
retrieving revision 1.1.1.1.2.8
diff -u -r1.1.1.1.2.8 m4.h
--- src/m4.h    6 Jun 2006 13:20:57 -0000       1.1.1.1.2.8
+++ src/m4.h    15 Jun 2006 21:44:37 -0000
@@ -480,3 +480,8 @@
 # define DEBUG_SYM
 # define DEBUG_INCL
 #endif
+
+/* Convert a possibly-signed character to an unsigned character.  This is
+   a bit safer than casting to unsigned char, since it catches some type
+   errors that the cast doesn't.  */
+static inline unsigned char to_uchar (char ch) { return ch; }
Index: src/macro.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/macro.c,v
retrieving revision 1.1.1.1.2.3
diff -u -r1.1.1.1.2.3 macro.c
--- src/macro.c 6 Jun 2006 13:20:57 -0000       1.1.1.1.2.3
+++ src/macro.c 15 Jun 2006 21:44:37 -0000
@@ -123,7 +123,7 @@
     {
       t = next_token (&td);
     }
-  while (t == TOKEN_SIMPLE && isspace (*TOKEN_DATA_TEXT (&td)));
+  while (t == TOKEN_SIMPLE && isspace (to_uchar (*TOKEN_DATA_TEXT (&td))));
 
   paren_level = 0;
 

--
View this message in context: 
http://www.nabble.com/FYI%3A-branch-1_4-character-traits-t1794913.html#a4891152
Sent from the Gnu - M4 - Patches forum at Nabble.com.