bugGNU indent - Bugs: bug #64503, heap-buffer-overflow overwrite at...

 
 

bug #64503: heap-buffer-overflow overwrite at indent.c:231

Submitter:  Yuchuan Meng <yisumi>
Submitted:  Wed 02 Aug 2023 02:55:40 AM UTC
   
 
Category:  C++ Severity:  3 - Normal
Item Group:  Crash Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 05 Oct 2023 05:58:34 PM UTC, comment #12: 

Will do. I did run the full test suite with the sanitizer enabled and `else-comment-2-br.c` also usually fails.

 Robert Scott <ris>
Thu 05 Oct 2023 11:11:22 AM UTC, comment #11: 

I see. I got confused with "your own test" and then tried a wrong file.

You are right that "./src/indent -npro -br -ce regression/input/else-comment-2.c -o /dev/null" command triggers a sanitizer abort even with the two patches.

Did you try to run all existing tests with a sanitizer? I also did so. (If I remember correctly, it's the only test failing.) But your finding (a buffer overread in current_column()) is a different bug from this bug report and subject of the two patches.  The bug existed before the two patches and still exist in the code.

Feel free to report a new bug.

Petr Pisar <petrp>
Wed 04 Oct 2023 08:00:52 PM UTC, comment #10: 

comment #9:

> $ ./src/indent -npro -br -ce regression/standard/else-comment-2-br-ce.c -o /tmp/out


It's not just that you're running it against the "standard" file instead of the test's input is it? (regression/input/else-comment-2.c)

This is the Nix package for indent more or less as at https://github.com/NixOS/nixpkgs/pull/256880. I've also got a version where I'm running it against vanilla 2685cc0bef0200733b634932ea7399b6cf91b6d7 with only the mentioned locale.h fix added (no fetchpatch shenanigans with the CVE patches that could have introduced newline differences in the test files) and it still shows the same effect.

This is on linux with gcc 12.3.0, and using NIX_DEBUG to show the full set of flags sent to gcc for e.g. indent.c:


extra flags before to /nix/store/n8dryz4xf7ln028j37zapgzg4j47p743-gcc-12.3.0/bin/gcc:
  -fPIC
  -fstack-protector-strong
  --param
  ssp-buffer-size=4
  -fno-strict-overflow
original flags to /nix/store/n8dryz4xf7ln028j37zapgzg4j47p743-gcc-12.3.0/bin/gcc:
  -DHAVE_CONFIG_H
  -DLOCALEDIR=\"/nix/store/ys3zj3r5i4yfv3grabvj0j4sdlzgw8jb-indent-2.2.13/share/locale\"
  -I.
  -I..
  -I.
  -I.
  -I../intl
  -Wall
  -W
  -Wredundant-decls
  -Wshadow
  -Wstrict-prototypes
  -Wmissing-prototypes
  -Wnested-externs
  -Wmissing-declarations
  -Wcomment
  -Wbad-function-cast
  -Wcast-align
  -g
  -O2
  -c
  -o
  indent.o
  indent.c
extra flags after to /nix/store/n8dryz4xf7ln028j37zapgzg4j47p743-gcc-12.3.0/bin/gcc:
  -B/nix/store/aw2fw9ag10wr9pf0qk4nk5sxi0q0bn56-glibc-2.37-8/lib/
  -idirafter
  /nix/store/4lap45ifj50lp980id60fnbvs81yj9lr-glibc-2.37-8-dev/include
  -idirafter
  /nix/store/n8dryz4xf7ln028j37zapgzg4j47p743-gcc-12.3.0/lib/gcc/x86_64-unknown-linux-gnu/12.3.0/include-fixed
  -B/nix/store/xpxln7rqi3pq4m0xpnawhxb2gs0mn1s0-gcc-12.3.0-lib/lib
  -B/nix/store/75slks1wr3b3sxr5advswjzg9lvbv9jc-gcc-wrapper-12.3.0/bin/
  -isystem
  /nix/store/2556v76mb7wdjhfmw3s8shx4mnkp62h4-gettext-0.21.1/include
  -isystem
  /nix/store/nxz0vkhska9x8r74kn8ahz032w5qhd5r-libtool-2.4.7/include
  -isystem
  /nix/store/dhpbypwjk5g11grq5zfqca2ymzyfgk3f-file-5.45-dev/include
  -O0
  -fsanitize=address
  -frandom-seed=ys3zj3r5i4


 Robert Scott <ris>
Wed 04 Oct 2023 12:30:10 PM UTC, comment #9: 

I cannot reproduce it. When I take current git master (2685cc0bef0200733b634932ea7399b6cf91b6d7) (and apply a fix for locale.h <https://savannah.gnu.org/bugs/index.php?64744>), configure with "./configure --disable-nls 'CFLAGS=-g -O0 -fsanitize=address'" and build with GCC 13.2.1, I do not observe any sanitizer abort:

$ ./src/indent -npro -br -ce regression/standard/else-comment-2-br-ce.c -o /tmp/out

Could you tell me what compiler and sanitizer you use and how you invoke indent?

Petr Pisar <petrp>
Sat 23 Sep 2023 11:38:28 PM UTC, comment #8: 

I don't think this is a complete fix. Running your own test `else-comment-2-br-ce` with asan enabled (on 2.2.13 with both patches applied) results in an over-read:


==3318385==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61200000014e at pc 0x0000004146c8 bp 0x7fffffffb550 sp 0x7fffffffb548
READ of size 1 at 0x61200000014e thread T0
    #0 0x4146c7 in current_column /build/indent-2.2.13/src/code_io.c:162
    #1 0x4091b4 in print_comment /build/indent-2.2.13/src/comments.c:298
    #2 0x41bc34 in handle_token_comment /build/indent-2.2.13/src/handletoken.c:2189
    #3 0x41bc34 in handle_the_token /build/indent-2.2.13/src/handletoken.c:2385
    #4 0x404e87 in indent_main_loop /build/indent-2.2.13/src/indent.c:672
    #5 0x404e87 in indent /build/indent-2.2.13/src/indent.c:759
    #6 0x402dd1 in indent_single_file /build/indent-2.2.13/src/indent.c:1004
    #7 0x402dd1 in indent_all /build/indent-2.2.13/src/indent.c:1042
    #8 0x402dd1 in main /build/indent-2.2.13/src/indent.c:1123
    #9 0x7ffff775bacd in __libc_start_call_main (/nix/store/aw2fw9ag10wr9pf0qk4nk5sxi0q0bn56-glibc-2.37-8/lib/libc.so.6+0x23acd)
    #10 0x7ffff775bb88 in __libc_start_main_alias_2 (/nix/store/aw2fw9ag10wr9pf0qk4nk5sxi0q0bn56-glibc-2.37-8/lib/libc.so.6+0x23b88)
    #11 0x4034b4 in _start (/tmp/nix-build-indent-2.2.13.drv-0/indent-2.2.13/src/indent+0x4034b4)

0x61200000014e is located 0 bytes to the right of 270-byte region [0x612000000040,0x61200000014e)
allocated by thread T0 here:
    #0 0x7ffff79d9de7 in __interceptor_calloc (/nix/store/xpxln7rqi3pq4m0xpnawhxb2gs0mn1s0-gcc-12.3.0-lib/lib/libasan.so.8+0xbbde7)
    #1 0x4142ec in xmalloc /build/indent-2.2.13/src/globs.c:42

SUMMARY: AddressSanitizer: heap-buffer-overflow /build/indent-2.2.13/src/code_io.c:162 in current_column
Shadow bytes around the buggy address:
  0x0c247fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff8000: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fff8010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c247fff8020: 00 00 00 00 00 00 00 00 00[06]fa fa fa fa fa fa
  0x0c247fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8070: 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
==3318385==ABORTING


on both macos and linux systems for me.

 Robert Scott <ris>
Thu 14 Sep 2023 09:23:19 AM UTC, comment #7: 

Patches applied, thanks a lot!

 Andrej Shadura <andrewsh>
Group administrator
Wed 16 Aug 2023 02:33:28 PM UTC, comment #6: 

Scratch my last comment.

I have a fix.

indent-2.2.13-Fix-an-out-of-buffer-read-in-search_brace-lexi-on-an.patch fixes the "short" reproducer. It's the same as 0001-Fix-an-out-of-buffer-read-in-search_brace-lexi-on-an.patch, but it includes a test now. This issue is a not exactly what the reporter reported.

indent-2.2.13-Fix-a-heap-buffer-overwrite-in-search_brace-CVE-2023.patch fixes the original reproducer, as well as "write1" and "read2" reproducers. Solely this patch is enough to fix the original bug. This bug is now known as CVE-2023-40305.

You can use valgrind or building with "gcc -fsanitize" to reproduce both issues.

Please review and apply both patches as each of them fix an independent issue.


(file #55049, file #55050)

 Petr Pisar <petrp>
Tue 15 Aug 2023 04:41:15 PM UTC, comment #5: 

An example of the second scenario is in "case comment" branch of search_brace() function:

                    need_chars (&save_com, 2);
                    *save_com.end = *buf_ptr++;
                    save_com.len++;

Petr Pisar <petrp>
Tue 15 Aug 2023 04:09:20 PM UTC, comment #4: 

I suspect two scenarios:

Either "need_chars (&save_com, 10)" in search_brace() does not preallocate enough characters for "*save_com.end++ = '{'" at indent.c:232,

or sw_buffer() can keep a dangling pointer if need_chars() moves a a buffer with xrealloc().

The second scenario would explain why this bug needs overlong comments aligned to specific length, triggering the reallocation.

 Petr Pisar <petrp>
Tue 15 Aug 2023 04:01:17 PM UTC, comment #3: 

Injecting two newlines into "read2" (see "write1") leads to a buffer overwrite. This overwrite is on a next line, indent.c:232, to a place reported originally, indent.c:231.

(file #55042)

 Petr Pisar <petrp>
Tue 15 Aug 2023 03:55:10 PM UTC, comment #2: 

But there is another buffer overread. See "read2" reproducer which simply replaces the last "x" character with "{".

(file #55041)

 Petr Pisar <petrp>
Tue 15 Aug 2023 03:52:43 PM UTC, comment #1: 

The "short" attachment is actually a different bug, a buffer overread.

(file #55040)

 Petr Pisar <petrp>
Wed 02 Aug 2023 02:55:40 AM UTC, original submission:  

### Description
heap-buffer-overflow overwrite at indent.c:231

### Version
```
GNU indent 2.2.13
```

### Replay
```
cd indent-2.2.13
make clean
./configure --disable-shared
make -j$(nproc)
indent ./poc
```

### POC
[poc](https://paste.debian.net/download/1287639)

### ASAN
```
➜  indent/indent -o test.c 1287639
=================================================================
==3843480==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61d000001280 at pc 0x55555556ee90 bp 0x7fffffffdc80 sp 0x7fffffffdc70
WRITE of size 1 at 0x61d000001280 thread T0
    #0 0x55555556ee8f in search_brace     indent-2.2.13/src/indent.c:231
    #1 0x55555556ee8f in indent_main_loop     indent-2.2.13/src/indent.c:548
    #2 0x5555555680a4 in indent     indent-2.2.13/src/indent.c:758
    #3 0x5555555680a4 in indent_single_file     indent-2.2.13/src/indent.c:1003
    #4 0x5555555680a4 in indent_all     indent-2.2.13/src/indent.c:1041
    #5 0x5555555680a4 in main     indent-2.2.13/src/indent.c:1122
    #6 0x7ffff702c082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082)
    #7 0x55555556938d in _start (   indent/indent+0x1538d)

0x61d000001280 is located 0 bytes to the right of 2048-byte region [0x61d000000a80,0x61d000001280)
allocated by thread T0 here:
    #0 0x7ffff72d9d40 in realloc (/lib/x86_64-linux-gnu/libasan.so.4+0xdfd40)
    #1 0x555555595d32 in xrealloc     indent-2.2.13/src/globs.c:64

SUMMARY: AddressSanitizer: heap-buffer-overflow     indent-2.2.13/src/indent.c:231 in search_brace
Shadow bytes around the buggy address:
  0x0c3a7fff8200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a7fff8210: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a7fff8220: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a7fff8230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a7fff8240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c3a7fff8250:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a7fff8260: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a7fff8270: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a7fff8280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a7fff8290: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a7fff82a0: 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
==3843480==ABORTING
```

### Environment
```
Linux server 5.4.0-153-generic #170-Ubuntu SMP Fri Jun 16 13:43:31 UTC 2023 x86_64 x86_64 x86_64 GNU/Linu
gcc (Ubuntu 7.5.0-6ubuntu2) 7.5.0
```

### Credit
([Fudan University](https://secsys.fudan.edu.cn/))

 Yuchuan Meng <yisumi>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #55049:  indent-2.2.13-Fix-an-out-of-buffer-read-in-search_brace-lexi-on-an.patch added by petrp (12KiB - text/x-patch)
file #55050:  indent-2.2.13-Fix-a-heap-buffer-overwrite-in-search_brace-CVE-2023.patch added by petrp (14KiB - text/x-patch)
file #55042:  write1 added by petrp (2KiB - application/octet-stream)
file #55041:  read2 added by petrp (2KiB - application/octet-stream)
file #55040:  0001-Fix-an-out-of-buffer-read-in-search_brace-lexi-on-an.patch added by petrp (3KiB - text/x-patch - Fix for a "short" reproducer)
file #55038:  short added by petrp (2KiB - application/octet-stream - Even shorter reproducer)
file #55035:  short added by petrp (2KiB - application/octet-stream - A minimized reproducer)
file #55036:  short added by petrp (2KiB - application/octet-stream - A minimized reproducer)
file #55006:  id:000317,sig:06,src:006047+005530,op:MOpt-core-splice,rep:32 added by yisumi (6KiB - application/octet-stream - poc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ris (Posted a comment)
  • -email is unavailable- added by andrewsh (Posted a comment)
  • -email is unavailable- added by petrp (Updated the item)
  • -email is unavailable- added by yisumi (Submitted the item)

    •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-09-14 andrewsh StatusNone Fixed
        Open/ClosedOpen Closed
    2023-08-16 petrp Attached File- Added indent-2.2.13-Fix-an-out-of-buffer-read-in-search_brace-lexi-on-an.patch, #55049
        Attached File- Added indent-2.2.13-Fix-a-heap-buffer-overwrite-in-search_brace-CVE-2023.patch, #55050
    2023-08-15 petrp Attached File- Added write1, #55042
    2023-08-15 petrp Attached File- Added read2, #55041
    2023-08-15 petrp Attached File- Added 0001-Fix-an-out-of-buffer-read-in-search_brace-lexi-on-an.patch, #55040
    2023-08-15 petrp Attached File- Added short, #55038
    2023-08-14 petrp Attached File- Added short, #55035
        Attached File- Added short, #55036
    2023-08-02 yisumi Attached File- Added id:000317,sig:06,src:006047+005530,op:MOpt-core-splice,rep:32, #55006

    Back to the top

    Copyright © 2024 Free Software Foundation, Inc.
    Verbatim copying and distribution of this entire article is permitted in any medium, provided this notice is preserved.
    The Levitating, Meditating, Flute-playing Gnu logo is a GNU GPL'ed image provided by the Nevrax Design Team.
    Page source code

    Powered by Savane 3.13-ec04.
    Corresponding source code