Skip to content

optarg length in -e command line argument is not checked what lead to overflow#368

Closed
ahpaleus wants to merge 1 commit intoopenssh:masterfrom
ahpaleus:heap-overflow-fix
Closed

optarg length in -e command line argument is not checked what lead to overflow#368
ahpaleus wants to merge 1 commit intoopenssh:masterfrom
ahpaleus:heap-overflow-fix

Conversation

@ahpaleus
Copy link

@ahpaleus ahpaleus commented Dec 29, 2022

Hello,

Heap buffer overflow occurs when an escape character is not provided in the -e CLI argument.

For the following command in the ssh binary of OpenSSH portable:

$ ./ssh -e^

the optarg's length is 1, so accessing optarg[2] is out of the allocated memory region. The argv is allocated on the heap here:

saved_av[i] = xstrdup(av[i]);

Address Sanitizer log:

$ ./ssh -e^
=================================================================
==1660986==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000054 at pc 0x0000004cdea0 bp 0x7ffe2fdf59f0 sp 0x7ffe2fdf59e8
READ of size 1 at 0x602000000054 thread T0
    #0 0x4cde9f in main /home/ssh_test/openssh/ssh.c:924:34
    #1 0x7f374a618082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    #2 0x4213fd in _start (/home/ssh_test/openssh/ssh+0x4213fd)

0x602000000054 is located 0 bytes to the right of 4-byte region [0x602000000050,0x602000000054)
allocated by thread T0 here:
    #0 0x499b3d in malloc (/home/ssh_test/openssh/ssh+0x499b3d)
    #1 0x640a3a in rpl_malloc /home/ssh_test/openssh/openbsd-compat/bsd-malloc.c:31:9
    #2 0x5c0a9a in xmalloc /home/ssh_test/openssh/xmalloc.c:40:8
    #3 0x5c0d55 in xstrdup /home/ssh_test/openssh/xmalloc.c:93:7
    #4 0x4cc7d2 in main /home/ssh_test/openssh/ssh.c:663:17
    #5 0x7f374a618082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/ssh_test/openssh/ssh.c:924:34 in main
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa 04 fa fa fa 06 fa fa fa[04]fa fa fa 00 00
  0x0c047fff8010: fa fa 00 07 fa fa 00 07 fa fa 00 05 fa fa 00 04
  0x0c047fff8020: fa fa 00 fa fa fa 00 fa fa fa 00 04 fa fa 00 00
  0x0c047fff8030: fa fa 00 00 fa fa 07 fa fa fa 02 fa fa fa 07 fa
  0x0c047fff8040: fa fa 00 05 fa fa 00 02 fa fa fd fa fa fa fd fa
  0x0c047fff8050: fa fa 00 fa fa fa 00 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
  Shadow gap:              cc
==1660986==ABORTING

@djmdjm
Copy link
Contributor

djmdjm commented Oct 18, 2024

Thanks; fixed in a slightly different way in d01ee7a

@djmdjm djmdjm closed this Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants