[Bug binutils/34759] New: strings -U {locale,escape,...} crashes with a stack overflow on inputs with many strings

[martin.mirchev at u dot nus.edu]

https://sourceware.org/bugzilla/show_bug.cgi?id=34759

            Bug ID: 34759
           Summary: strings -U {locale,escape,...} crashes with a stack
                    overflow on inputs with many strings
           Product: binutils
           Version: 2.48 (HEAD)
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: binutils
          Assignee: unassigned at sourceware dot org
          Reporter: martin.mirchev at u dot nus.edu
  Target Milestone: ---

With any -U mode other than "default", strings recurses once per string it
prints: print_unicode_stream_body() (files, stdin) and print_unicode_buffer()
(section contents) end with a tail call to themselves, marked with FIXME
comments. GCC does not turn these into loops, even at -O2, so the stack
depth grows with the number of strings and a few tens of thousands of
strings exhaust the default 8 MiB stack:

$ wc -l /usr/share/dict/words
102401 /usr/share/dict/words
$ strings -U escape /usr/share/dict/words > /dev/null
Segmentation fault (core dumped)
$ strings -U locale /usr/share/dict/words > /dev/null
Segmentation fault (core dumped)

The same happens for large log files or any data with more than about
80,000 strings (e.g. printf 'AAAA\0' repeated 80000 times); -U default and
smaller inputs are fine. Seen with binutils 2.47 built with -O2 on x86_64.

Suggested fix: turn both tail calls into loops (update the parameters and
jump back to the start of the function), so the stack depth no longer
depends on the number of strings:

--- a/binutils/strings.c
+++ b/binutils/strings.c
@@ -810,6 +810,9 @@
       return;
     }

+  /* Each iteration displays at most one string and then continues with the
+     rest of the buffer.  */
+ next_string:
   if (buflen == 0)
     return;

@@ -900,8 +903,12 @@
   else
     putchar ('\n');

-  /* FIXME: Using tail recursion here is lazy programming...  */
-  print_unicode_buffer (filename, address + i, buffer + i, buflen - i);
+  /* Continue with the rest of the buffer.  Looping rather than recursing
+     keeps the stack depth independent of the number of strings.  */
+  address += i;
+  buffer += i;
+  buflen -= i;
+  goto next_string;
 }

 static int
@@ -940,6 +947,7 @@
      might be huge or it might time-locked (eg stdin).  So instead
      we go one byte at a time...  */

+ next_string:;
   file_ptr start_point = 0;
   unsigned int num_read = 0;
   unsigned int num_chars = 0;
@@ -1205,8 +1213,13 @@
     }

   if (c != EOF)
-    /* FIXME: Using tail recursion here is lazy, but it works.  */
-    print_unicode_stream_body (filename, address + num_read, stream,
putback_buf, num_putback, print_buf);
+    {
+      /* Continue with the rest of the stream.  Looping rather than
+        recursing keeps the stack depth independent of the number of
+        strings.  */
+      address += num_read;
+      goto next_string;
+    }
 }

 /* Display strings read in from STREAM.  Treat any UTF-8 encoded characters

With it the examples above complete, and the output is byte-identical to the
unpatched strings (run with an unlimited stack) for -U default, locale,
escape, invalid, hex and highlight, on files with -a, on stdin, and on ELF
section contents without -a.

This issue was found by an LLM-Enhanced Static Analysis Engine and verified
manually.

-- 
You are receiving this mail because:
You are on the CC list for the bug.