Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[BUG][FIX] Buffer Overflow caused by long ESSID #525

Closed
wants to merge 1 commit into from

Conversation

raefko
Copy link

@raefko raefko commented Mar 21, 2024

Buffer Overflow Caused by Long ESSID (>30 chars) and Call to sprintf

Description:

I've encountered a buffer overflow issue in the file output.c. The problem arises when the ESSID length exceeds 30 characters. The code attempts to call sprintf from a buffer of size 32 to a buffer of size 30, which can lead to an overflow if the ESSID is longer than 30 characters.

Steps to reproduce:

  1. Set the ESSID of your network to a string longer than 30 characters.
  2. Run the program.

Expected behavior:

The program should handle ESSIDs of any length without causing a buffer overflow.

Actual behavior:

The program causes a buffer overflow when the ESSID is longer than 30 characters.

==115654==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7efc05d0023e at pc 0x7efc08696fd6 bp 0x7ffc7e17fc60 sp 0x7ffc7e17f3f0
WRITE of size 31 at 0x7efc05d0023e thread T0
    #0 0x7efc08696fd5 in __interceptor_vsnprintf ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1746
    #1 0x7efc0869723e in __interceptor_snprintf ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1817
    #2 0x55d0ae1ffe2e in maybe_escape_markup ../src/output.c:93
    #3 0x55d0ae21ae24 in print_wireless_info ../src/print_wireless_info.c:607
    #4 0x55d0ae1f6c20 in main ../i3status.c:709
    #5 0x7efc08023a8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #6 0x7efc08023b48 in __libc_start_main_impl ../csu/libc-start.c:360
    #7 0x55d0ae1eac94 in _start (/home/raefko/i3status/build/i3status+0x12c94) (BuildId: 672c2796da52236def443eeaefd16a360cbb168f)

Address 0x7efc05d0023e is located in stack of thread T0 at offset 574 in frame
    #0 0x55d0ae218e2a in print_wireless_info ../src/print_wireless_info.c:513

  This frame has 10 object(s):
    [48, 56) 'tmp' (line 604)
    [80, 168) 'info' (line 516)
    [208, 320) 'placeholders' (line 623)
    [352, 382) 'string_quality' (line 569)
    [416, 446) 'string_signal' (line 570)
    [480, 510) 'string_noise' (line 571)
    [544, 574) 'string_essid' (line 572) <== Memory access at offset 574 overflows this variable
    [608, 638) 'string_frequency' (line 573)
    [672, 702) 'string_ip' (line 574)
    [736, 766) 'string_bitrate' (line 575)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1746 in __interceptor_vsnprintf
Shadow bytes around the buggy address:
  0x7efc05cfff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7efc05d00000: f1 f1 f1 f1 f1 f1 00 f2 f2 f2 00 00 00 00 00 00
  0x7efc05d00080: 00 00 00 00 00 f2 f2 f2 f2 f2 00 00 00 00 00 00
  0x7efc05d00100: 00 00 00 00 00 00 00 00 f2 f2 f2 f2 00 00 00 06
  0x7efc05d00180: f2 f2 f2 f2 00 00 00 06 f2 f2 f2 f2 00 00 00 06
=>0x7efc05d00200: f2 f2 f2 f2 00 00 00[06]f2 f2 f2 f2 00 00 00 06
  0x7efc05d00280: f2 f2 f2 f2 00 00 00 06 f2 f2 f2 f2 00 00 00 06
  0x7efc05d00300: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
  0x7efc05d00380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7efc05d00400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7efc05d00480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==115654==ABORTING

Environment:

Ubuntu 23.04
i3status commit : c07b9ca

Additional context:

The problematic code is located in

*buffer += sprintf(*buffer, "%s", text);

The max essid size is set here

#define IW_ESSID_MAX_SIZE 32

char essid[IW_ESSID_MAX_SIZE + 1];

But the buffer used in the sprintf function is set and defined here

#define STRING_SIZE 30

char string_essid[STRING_SIZE] = {'\0'};

char *tmp = string_essid;

Fix
My fix is to replace sprintf function by snprintf function with STRING_SIZE = 30

orestisfl added a commit that referenced this pull request May 5, 2024
This fixes #492 and an additional buffer overflow that can happen when
pango markup is enabled.

Using config
```
general {
        output_format = "none"
        markup = "pango"
}

order += "wireless _first_"

wireless _first_ {
  format_up = "W: (%quality at %essid, %bitrate) %ip"
}
```

and renaming my phone's hotspot to `Hello world &<<<<<<hello world>>`
i3status will throw an AddressSanitizer error:
```
==1373240==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7411d720923e at pc 0x7411daa7cee9 bp 0x7ffdae6ce070 sp 0x7ffdae6cd800
WRITE of size 5 at 0x7411d720923e thread T0
    #0 0x7411daa7cee8 in __interceptor_vsprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1765
    #1 0x7411daa7d0ff in __interceptor_sprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1808
    #2 0x653b2764cdaf in maybe_escape_markup ../src/output.c:102
    #3 0x653b27677df9 in print_wireless_info ../src/print_wireless_info.c:607
    #4 0x653b27640bf1 in main ../i3status.c:709
    #5 0x7411da641ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: 6542915cee3354fbcf2b3ac5542201faec43b5c9)
    #6 0x7411da641d89 in __libc_start_main (/usr/lib/libc.so.6+0x25d89) (BuildId: 6542915cee3354fbcf2b3ac5542201faec43b5c9)
    #7 0x653b27633f24 in _start (/tmp/xx/i3status/build/i3status+0x4ff24) (BuildId: c737ce6288265fa02a7617c66f51ddd16b5a8275)

Address 0x7411d720923e is located in stack of thread T0 at offset 574 in frame
    #0 0x653b276750ed in print_wireless_info ../src/print_wireless_info.c:513

  This frame has 10 object(s):
    [48, 56) 'tmp' (line 604)
    [80, 168) 'info' (line 516)
    [208, 320) 'placeholders' (line 623)
    [352, 382) 'string_quality' (line 569)
    [416, 446) 'string_signal' (line 570)
    [480, 510) 'string_noise' (line 571)
    [544, 574) 'string_essid' (line 572) <== Memory access at offset 574 overflows this variable
    [608, 638) 'string_frequency' (line 573)
    [672, 702) 'string_ip' (line 574)
    [736, 766) 'string_bitrate' (line 575)
```

With pango disabled, the error is thrown elsewhere (#492):
```
==1366779==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7bab43a0923e at pc 0x7bab4727cee9 bp 0x7ffc289d2540 sp 0x7ffc289d1cd0
WRITE of size 33 at 0x7bab43a0923e thread T0
    #0 0x7bab4727cee8 in __interceptor_vsprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1765
    #1 0x7bab4727d0ff in __interceptor_sprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1808
    #2 0x5dd180858aa4 in maybe_escape_markup ../src/output.c:93
    #3 0x5dd180883df9 in print_wireless_info ../src/print_wireless_info.c:607
    #4 0x5dd18084cbf1 in main ../i3status.c:709
    #5 0x7bab46843ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: 6542915cee3354fbcf2b3ac5542201faec43b5c9)
    #6 0x7bab46843d89 in __libc_start_main (/usr/lib/libc.so.6+0x25d89) (BuildId: 6542915cee3354fbcf2b3ac5542201faec43b5c9)
    #7 0x5dd18083ff24 in _start (/tmp/xx/i3status/build/i3status+0x4ff24) (BuildId: c737ce6288265fa02a7617c66f51ddd16b5a8275)

Address 0x7bab43a0923e is located in stack of thread T0 at offset 574 in frame
    #0 0x5dd1808810ed in print_wireless_info ../src/print_wireless_info.c:513

  This frame has 10 object(s):
    [48, 56) 'tmp' (line 604)
    [80, 168) 'info' (line 516)
    [208, 320) 'placeholders' (line 623)
    [352, 382) 'string_quality' (line 569)
    [416, 446) 'string_signal' (line 570)
    [480, 510) 'string_noise' (line 571)
    [544, 574) 'string_essid' (line 572) <== Memory access at offset 574 overflows this variable
    [608, 638) 'string_frequency' (line 573)
    [672, 702) 'string_ip' (line 574)
    [736, 766) 'string_bitrate' (line 575)
```

With the patch output is correct:
```
W: ( 72% at Hello world &amp;&lt;&lt;&lt;&lt;&lt;&lt;hello world&gt;&gt;, 1,2009 Gb/s) 192.168.26.237
```
and
```
W: ( 73% at Hello world &<<<<<<hello world>>, 1,1342 Gb/s) 192.168.26.237
```

The patch changes the maybe_escape_markup function to use dynamic
allocation instead of a static buffer. Confusing pointer arithmetic is
replaced with index-based memory access. The `buffer` pointer does not
move around except for `realloc`ations.

Fixes #492
Closes #525 (alternative PR)
stapelberg pushed a commit that referenced this pull request May 8, 2024
* maybe_escape_markup: Make function memory-safe

This fixes #492 and an additional buffer overflow that can happen when
pango markup is enabled.

Using config
```
general {
        output_format = "none"
        markup = "pango"
}

order += "wireless _first_"

wireless _first_ {
  format_up = "W: (%quality at %essid, %bitrate) %ip"
}
```

and renaming my phone's hotspot to `Hello world &<<<<<<hello world>>`
i3status will throw an AddressSanitizer error:
```
==1373240==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7411d720923e at pc 0x7411daa7cee9 bp 0x7ffdae6ce070 sp 0x7ffdae6cd800
WRITE of size 5 at 0x7411d720923e thread T0
    #0 0x7411daa7cee8 in __interceptor_vsprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1765
    #1 0x7411daa7d0ff in __interceptor_sprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1808
    #2 0x653b2764cdaf in maybe_escape_markup ../src/output.c:102
    #3 0x653b27677df9 in print_wireless_info ../src/print_wireless_info.c:607
    #4 0x653b27640bf1 in main ../i3status.c:709
    #5 0x7411da641ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: 6542915cee3354fbcf2b3ac5542201faec43b5c9)
    #6 0x7411da641d89 in __libc_start_main (/usr/lib/libc.so.6+0x25d89) (BuildId: 6542915cee3354fbcf2b3ac5542201faec43b5c9)
    #7 0x653b27633f24 in _start (/tmp/xx/i3status/build/i3status+0x4ff24) (BuildId: c737ce6288265fa02a7617c66f51ddd16b5a8275)

Address 0x7411d720923e is located in stack of thread T0 at offset 574 in frame
    #0 0x653b276750ed in print_wireless_info ../src/print_wireless_info.c:513

  This frame has 10 object(s):
    [48, 56) 'tmp' (line 604)
    [80, 168) 'info' (line 516)
    [208, 320) 'placeholders' (line 623)
    [352, 382) 'string_quality' (line 569)
    [416, 446) 'string_signal' (line 570)
    [480, 510) 'string_noise' (line 571)
    [544, 574) 'string_essid' (line 572) <== Memory access at offset 574 overflows this variable
    [608, 638) 'string_frequency' (line 573)
    [672, 702) 'string_ip' (line 574)
    [736, 766) 'string_bitrate' (line 575)
```

With pango disabled, the error is thrown elsewhere (#492):
```
==1366779==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7bab43a0923e at pc 0x7bab4727cee9 bp 0x7ffc289d2540 sp 0x7ffc289d1cd0
WRITE of size 33 at 0x7bab43a0923e thread T0
    #0 0x7bab4727cee8 in __interceptor_vsprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1765
    #1 0x7bab4727d0ff in __interceptor_sprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1808
    #2 0x5dd180858aa4 in maybe_escape_markup ../src/output.c:93
    #3 0x5dd180883df9 in print_wireless_info ../src/print_wireless_info.c:607
    #4 0x5dd18084cbf1 in main ../i3status.c:709
    #5 0x7bab46843ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: 6542915cee3354fbcf2b3ac5542201faec43b5c9)
    #6 0x7bab46843d89 in __libc_start_main (/usr/lib/libc.so.6+0x25d89) (BuildId: 6542915cee3354fbcf2b3ac5542201faec43b5c9)
    #7 0x5dd18083ff24 in _start (/tmp/xx/i3status/build/i3status+0x4ff24) (BuildId: c737ce6288265fa02a7617c66f51ddd16b5a8275)

Address 0x7bab43a0923e is located in stack of thread T0 at offset 574 in frame
    #0 0x5dd1808810ed in print_wireless_info ../src/print_wireless_info.c:513

  This frame has 10 object(s):
    [48, 56) 'tmp' (line 604)
    [80, 168) 'info' (line 516)
    [208, 320) 'placeholders' (line 623)
    [352, 382) 'string_quality' (line 569)
    [416, 446) 'string_signal' (line 570)
    [480, 510) 'string_noise' (line 571)
    [544, 574) 'string_essid' (line 572) <== Memory access at offset 574 overflows this variable
    [608, 638) 'string_frequency' (line 573)
    [672, 702) 'string_ip' (line 574)
    [736, 766) 'string_bitrate' (line 575)
```

With the patch output is correct:
```
W: ( 72% at Hello world &amp;&lt;&lt;&lt;&lt;&lt;&lt;hello world&gt;&gt;, 1,2009 Gb/s) 192.168.26.237
```
and
```
W: ( 73% at Hello world &<<<<<<hello world>>, 1,1342 Gb/s) 192.168.26.237
```

The patch changes the maybe_escape_markup function to use dynamic
allocation instead of a static buffer. Confusing pointer arithmetic is
replaced with index-based memory access. The `buffer` pointer does not
move around except for `realloc`ations.

Fixes #492
Closes #525 (alternative PR)

* Revert to snprintf
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant