URL:
  <https://savannah.gnu.org/bugs/?68560>

                 Summary: [PATCH] [grops] ASAN reports 1-byte heap overread
with certain invalid `import` device extension command arguments
                   Group: GNU roff
               Submitter: gbranden
               Submitted: Fri 24 Jul 2026 11:44:40 PM UTC
                Category: Driver grops
                Severity: 4 - Important
              Item Group: Crash/Unresponsive
                  Status: In Progress
                 Privacy: Public
             Assigned to: gbranden
             Open/Closed: Open
         Discussion Lock: Unlocked
         Planned Release: None


    _______________________________________________________

Follow-up Comments:


-------------------------------------------------------
Date: Fri 24 Jul 2026 11:44:40 PM UTC By: G. Branden Robinson <gbranden>
Bruno Haible and I independently encountered this problem while hitting
_groff_ 1.25.0.rc1 with the ASAN hammer.

[https://lists.gnu.org/archive/html/groff/2026-07/msg00053.html His report to
the mailing list is well-organized.]


I tried building the groff-1.25.0.rc1 with clang's ASAN.
The next finding:

  GROFF    doc/webpage.ps
=================================================================
==850822==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x7667d11e0291 at pc 0x639af6237a11 bp 0x7ffd0e103b90 sp 0x7ffd0e103b88
READ of size 1 at 0x7667d11e0291 thread T0
    #0 0x639af6237a10 in ps_printer::do_import(char*, environment const*) 
/build/groff-1.25.0.rc1/build-64-clang/../src/devices/grops/ps.cpp:1870:11
    #1 0x639af6235d48 in ps_printer::special(char*, environment const*, char)

/build/groff-1.25.0.rc1/build-64-clang/../src/devices/grops/ps.cpp:1734:7
    #2 0x639af6253ef4 in parse_x_command() 
/build/groff-1.25.0.rc1/build-64-clang/../src/libs/libdriver/input.cpp:1530:6
    #3 0x639af625725f in interpret_troff_output_file(char const*) 
/build/groff-1.25.0.rc1/build-64-clang/../src/libs/libdriver/input.cpp:1827:17
    #4 0x639af623a810 in main 
/build/groff-1.25.0.rc1/build-64-clang/../src/devices/grops/ps.cpp:2053:5
    #5 0x7a07d1e2a600 in __libc_start_call_main 
csu/../sysdeps/nptl/libc_start_call_main.h:59:16
    #6 0x7a07d1e2a717 in __libc_start_main csu/../csu/libc-start.c:360:3
    #7 0x639af612ce34 in _start 
(/build/groff-1.25.0.rc1/build-64-clang/grops+0x85e34)

0x7667d11e0291 is located 0 bytes after 49-byte region 
[0x7667d11e0260,0x7667d11e0291)
allocated by thread T0 here:
    #0 0x639af62154fd in operator new[](unsigned long) 
/home/runner/work/llvm-project/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:111:37
    #1 0x639af624eb10 in StringBuf::make_string() 
/build/groff-1.25.0.rc1/build-64-clang/../src/libs/libdriver/input.cpp:568:18
    #2 0x639af625072c in get_extended_arg() 
/build/groff-1.25.0.rc1/build-64-clang/../src/libs/libdriver/input.cpp:773:14
    #3 0x639af6253d26 in parse_x_command() 
/build/groff-1.25.0.rc1/build-64-clang/../src/libs/libdriver/input.cpp:1523:23
    #4 0x639af625725f in interpret_troff_output_file(char const*) 
/build/groff-1.25.0.rc1/build-64-clang/../src/libs/libdriver/input.cpp:1827:17
    #5 0x639af623a810 in main 
/build/groff-1.25.0.rc1/build-64-clang/../src/devices/grops/ps.cpp:2053:5
    #6 0x7a07d1e2a600 in __libc_start_call_main 
csu/../sysdeps/nptl/libc_start_call_main.h:59:16
    #7 0x7a07d1e2a717 in __libc_start_main csu/../csu/libc-start.c:360:3
    #8 0x639af612ce34 in _start 
(/build/groff-1.25.0.rc1/build-64-clang/grops+0x85e34)

SUMMARY: AddressSanitizer: heap-buffer-overflow 
/build/groff-1.25.0.rc1/build-64-clang/../src/devices/grops/ps.cpp:1870:11 in

ps_printer::do_import(char*, environment const*)
Shadow bytes around the buggy address:
  0x7667d11e0000: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
  0x7667d11e0080: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
  0x7667d11e0100: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
  0x7667d11e0180: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
  0x7667d11e0200: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
=>0x7667d11e0280: 00 00[01]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7667d11e0300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7667d11e0380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7667d11e0400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7667d11e0480: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7667d11e0500: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==850822==ABORTING
/build/groff-1.25.0.rc1/build-64-clang/groff: error: grops: Aborted
make[2]: *** [Makefile:19776: doc/webpage.ps] Error 8


"READ of size 1" with "is located 0 bytes after 49-byte region" obviously
means that the memory block should have been 50 bytes large, not 49.

Bruno


I believe this fixes it:


diff --git a/ChangeLog b/ChangeLog
index f134a619b..3182b8095 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2026-07-24  G. Branden Robinson <[email protected]>
+
+       * src/devices/grops/ps.cpp (ps_printer::do_import): Revise to
+       avoid overreading heap by one byte with certain ill-formed
+       argument to the `import` device extension command.  Recast
+       error diagnostic to report the full contents of the invalid
+       argument and characterize it simply as "invalid" due to
+       non-numeric contents.
+
+       Fixes <https://savannah.gnu.org/bugs/?68???>.  Problem
+       introduced by me in commit 9f050b7ed7, 7 May.
+
 2026-07-24  G. Branden Robinson <[email protected]>
 
        * src/devices/grops/ps.cpp (ps_printer::do_import): Fix
diff --git a/src/devices/grops/ps.cpp b/src/devices/grops/ps.cpp
index d937d3615..755e34450 100644
--- a/src/devices/grops/ps.cpp
+++ b/src/devices/grops/ps.cpp
@@ -1852,6 +1852,8 @@ void ps_printer::do_import(char *arg, const environment
*env)
   while ((' ' == *arg) || ('\n' == *arg))
     arg++;
   char *p;
+  // Skip validation of first argument, a file name.
+  // XXX: This means we can't import file names with spaces in them.
   for (p = arg; (*p != '\0') && (*p != ' ') && (*p != '\n'); p++)
     ;
   if (*p != '\0')
@@ -1866,11 +1868,15 @@ void ps_printer::do_import(char *arg, const
environment *env)
     parms[nparms++] = int(n);
     p = end;
   }
-  if ((csalpha(*p) && (p[1] == '\0'))
-      || (p[1] == ' ')
-      || (p[1] == '\n')) {
-    error("scaling unit '%1' not allowed in argument to"
-         "device extension command 'import'", p[1]);
+  size_t idx = 0;
+  if (strlen(p) > 1)
+    idx = 1;
+  if (!csdigit(p[idx])
+      && (p[idx] != ' ')
+      && (p[idx] != '\n')
+      && (p[idx] != '\0')) {
+    error("invalid numeric argument(s) '%1'"
+         " to device extension command 'import'", p);
     return;
   }
   while ((' ' == *p) || ('\n' == *p))









    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?68560>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/

Attachment: signature.asc
Description: PGP signature

Reply via email to