Bug 33046 - SEGV in bzip2.c:932
Summary: SEGV in bzip2.c:932
Status: RESOLVED FIXED
Alias: None
Product: bzip2
Classification: Unclassified
Component: bzip2 (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-06-02 15:22 UTC by Виталий Листратенко
Modified: 2025-06-19 19:14 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
Project(s) to access:
ssh public key:


Attachments
Input file (622 bytes, application/octet-stream)
2025-06-02 15:22 UTC, Виталий Листратенко
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Виталий Листратенко 2025-06-02 15:22:56 UTC
Created attachment 16115 [details]
Input file

Overview.
I have found a SEGV bug while fuzzing bzip2 utility.

The bug appears in copyFileName() function in bzip2.c file at line 932:

void copyFileName ( Char* to, Char* from ) 
{
   if ( strlen(from) > FILE_NAME_LEN-10 )  {

and at line 944:

strncpy(to,from,FILE_NAME_LEN-10);

The bug happens because second argument 'from' points to NULL.

Possible solution for this bug is check argument 'from' before using:

diff --git a/bzip2.c b/bzip2.c
index 608df64..4be00a1 100644
--- a/bzip2.c
+++ b/bzip2.c
@@ -918,7 +918,7 @@ void pad ( Char *s )
 static 
 void copyFileName ( Char* to, Char* from ) 
 {
-   if ( strlen(from) > FILE_NAME_LEN-10 )  {
+   if (from && strlen(from) > FILE_NAME_LEN-10 )  {
       fprintf (
          stderr,
          "bzip2: file name\n`%s'\n"
@@ -930,7 +930,8 @@ void copyFileName ( Char* to, Char* from )
       exit(exitValue);
    }
 
-  strncpy(to,from,FILE_NAME_LEN-10);
+  if (from)
+    strncpy(to,from,FILE_NAME_LEN-10);
   to[FILE_NAME_LEN-10]='\0';
 }
 

Steps to Reproduce.
1) Define afl_init_argv() function in bzip2.c file:

#define MAX_CMDLINE_LEN 100000
#define MAX_CMDLINE_PAR 50000

static char **afl_init_argv(int *argc) {

  static char  in_buf[MAX_CMDLINE_LEN];
  static char *ret[MAX_CMDLINE_PAR];

  char *ptr = in_buf;
  int   rc = 0;

  ssize_t num = read(0, in_buf, MAX_CMDLINE_LEN - 2);
  if (num < 1) { _exit(1); }
  in_buf[num] = '\0';
  in_buf[num + 1] = '\0';

  while (*ptr && rc < MAX_CMDLINE_PAR) {

    ret[rc] = ptr;
    if (ret[rc][0] == 0x02 && !ret[rc][1]) ret[rc]++;
    rc++;

    while (*ptr)
      ptr++;
    ptr++;

  }

  *argc = rc;

  return ret;

}

2) Call afl_init_argv() function in main() function here (near line 1850):

copyFileName ( inName,  (Char*)"(none)" );
copyFileName ( outName, (Char*)"(none)" );
argv = afl_init_argv(&argc);
copyFileName ( progNameReally, argv[0] );
progName = &progNameReally[0];

3) Build bzip2 using this flags:
export CC="clang"
export CFLAGS="-fsanitize=address,undefined"
make

4) Execute ./bzip2 < id:000014,sig:06,src:000610,time:5004092,execs:7919030,op:havoc,rep:35
(Input file from attachment)

Actual Results.
bzip2 crashed with UBSAN message:

bzip2.c:922:15: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:385:33: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior bzip2.c:922:15

Build 2 June 2025 on Astra Linux 1.7.7
bzip version 1.0.8
Comment 1 Mark Wielaard 2025-06-08 19:04:57 UTC
Could you show the full backtrace when this crashes?

As far as I can see copyFileName is always called with either inName, outName or progNameReally as first argument. All three are defined as file based char array variable, so cannot be NULL.

The second argument also cannot be NULL as far as I can see. If it was and srcMode != SM_I2O then libbz would call panic. Otherwise (srcMode == SM_I2O) it would be set to a static string.

A full backtrace would show where this exactly is called from (the line number, 922 also doesn't fit a copyFileName, probably because you changed the file?)
Comment 2 Виталий Листратенко 2025-06-09 11:06:07 UTC
Hi, Mark!
Here is the full backtrace:

#0 0x729cda76e141  (/lib/x86_64-linux-gnu/libc.so.6+0x15c141) (BuildId: #0 0x7701d1143141  (/lib/x86_64-linux-gnu/libc.so.6+0x15c141) (BuildId: 79cd7beb3903a9b34e306f52a988d970e13524a6)
    #1 0x63eded3e6513 in strlen (/bzip2-upstream/bzip2+0x4a513) (BuildId: e83e78600eb1460d7820a435a1fc236b3cf2b04f)
    #2 0x63eded4b5c08 in copyFileName /bzip2-upstream/bzip2.c:932:9
    #3 0x63eded4b2c51 in main /bzip2-upstream/bzip2.c:1830:4
    #4 0x7701d100b09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a) (BuildId: 79cd7beb3903a9b34e306f52a988d970e13524a6)
    #5 0x63eded3cd4a9 in _start (/bzip2-upstream/bzip2+0x314a9) (BuildId: e83e78600eb1460d7820a435a1fc236b3cf2b04f)

The second argument can be NULL as it reads from argv[0] in main() function (line 1828).

About line numbers - I tested bzip2 on different versions and copied UBSAN output from another version. But the bug is actual for last version. And the backtrace above I copied from the last version too. Also at the beginning of my report I wrote the actual line numbers.
Comment 3 Mark Wielaard 2025-06-15 13:21:45 UTC
OK, so the issue is that argv[0] might be NULL. I don't really know when that can occur or on which system. Does that mean argc is also zero?

But we can easily guard against it:

diff --git a/bzip2.c b/bzip2.c
index 9ef753649138..374015ebb862 100644
--- a/bzip2.c
+++ b/bzip2.c
@@ -1825,7 +1825,11 @@ IntNative main ( IntNative argc, Char *argv[] )
    copyFileName ( inName,  (Char*)"(none)" );
    copyFileName ( outName, (Char*)"(none)" );
 
-   copyFileName ( progNameReally, argv[0] );
+   if (argv[0] != NULL)
+      copyFileName ( progNameReally, argv[0] );
+   else
+      copyFileName ( progNameReally, (Char*)"bzip2" );
+
    progName = &progNameReally[0];
    for (tmp = &progNameReally[0]; *tmp != '\0'; tmp++)
       if (*tmp == PATH_SEP) progName = tmp + 1;

Would that solve your issue?
Comment 4 Sam James 2025-06-15 13:23:26 UTC
(In reply to Mark Wielaard from comment #3)
> OK, so the issue is that argv[0] might be NULL. I don't really know when
> that can occur or on which system. Does that mean argc is also zero?
> 

IIRC it can happen in some weird cases like with the simulators when running the GCC testsuite.
Comment 5 Виталий Листратенко 2025-06-16 11:36:04 UTC
(In reply to Mark Wielaard from comment #3)
> Would that solve your issue?

Yes, this solve the issue.

But I have notice that to init argv for fuzzing, the AFL_INIT_ARGV macro calls instead of AFL_INIT_SET0 macro.

The AFL_INIT_ARGV macro rewrite argv[0] to NULL and argc to zero.

But, like @Sam James said above, it can happens in some weird cases.

I'll appreciate if you fix this bug in upstream, but I'll understand if you don't.
Comment 6 Mark Wielaard 2025-06-19 19:14:48 UTC
commit af79253677ad98d6dfe11ea315ee9947d86586d3
Author: Mark Wielaard <mark@klomp.org>
Date:   Thu Jun 19 21:07:22 2025 +0200

    bzip2.c: Check argc >= 1 && argv[0] != NULL
    
    This should never happen, but if there is no, or a NULL argv[0] then
    use a hard coded string "bzip2" when calling copyFileName to define
    progNameReally.
    
    https://sourceware.org/bugzilla/show_bug.cgi?id=33046