[Home]

Summary:ASTERISK-24801: ASAN: ast_el_read_char stack-buffer-overflow
Reporter:Badalian Vyacheslav (slavon)Labels:
Date Opened:2015-02-17 11:47:29.000-0600Date Closed:2016-02-12 10:45:50.000-0600
Priority:MinorRegression?
Status:Closed/CompleteComponents:
Versions:11.15.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:stack-buffer-overflow found by ASAN.

It is difficult to replay. Clearly something with TAB in addition to CLI. I think if I type Russian characters and click TAB.

ASAN debug:
{code}
==30507==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff5322d6ff at pc 0x476c64 bp 0x7fff5322d620 sp 0x7fff5322d618
READ of size 1 at 0x7fff5322d6ff thread T0
   #0 0x476c63 in ast_el_read_char /root/asterisk-11.15.0/main/asterisk.c:2588
   #1 0x7831b9 in el_getc /root/asterisk-11.15.0/main/editline/read.c:350
   #2 0x786e6f in read_getcmd /root/asterisk-11.15.0/main/editline/read.c:243
   #3 0x786e6f in el_gets /root/asterisk-11.15.0/main/editline/read.c:446
   #4 0x47c316 in ast_remotecontrol /root/asterisk-11.15.0/main/asterisk.c:3182
   #5 0x42a652 in main /root/asterisk-11.15.0/main/asterisk.c:4029
   #6 0x7fd2ed7c3d5c in __libc_start_main (/lib64/libc.so.6+0x1ed5c)
   #7 0x42d304 (/usr/sbin/asterisk+0x42d304)

Address 0x7fff5322d6ff is located in stack of thread T0 at offset 95 in frame
   #0 0x47644f in ast_el_read_char /root/asterisk-11.15.0/main/asterisk.c:2513

 This frame has 2 object(s):
   [32, 48) 'fds'
   [96, 608) 'buf' <== Memory access at offset 95 underflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
     (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /root/asterisk-11.15.0/main/asterisk.c:2588 ast_el_read_char
Shadow bytes around the buggy address:
 0x10006a63da80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x10006a63da90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x10006a63daa0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x10006a63dab0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x10006a63dac0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10006a63dad0: 00 00 00 00 f1 f1 f1 f1 00 00 f4 f4 f2 f2 f2[f2]
 0x10006a63dae0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x10006a63daf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x10006a63db00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x10006a63db10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x10006a63db20: f3 f3 f3 f3 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
 Heap right redzone:      fb
 Freed heap region:       fd
 Stack left redzone:      f1
 Stack mid redzone:       f2
 Stack right redzone:     f3
 Stack partial redzone:   f4
 Stack after return:      f5
 Stack use after scope:   f8
 Global redzone:          f9
 Global init order:       f6
 Poisoned by user:        f7
 Contiguous container OOB:fc
 ASan internal:           fe
==30507==ABORTING

{code}
Comments:By: Badalian Vyacheslav (slavon) 2015-02-17 12:02:26.253-0600

{code}
                       if (res < 1) {
...
                               continue;
                       }
{code}

{code}
                       if ((res < EL_BUF_SIZE - 1) && ((buf[res-1] == '\n') || (buf[res-2] == '\n'))) {
{code}

if res = 1, then buf[res-2] is buffer overflow
By: Diederik de Groot (dkdegroot) 2016-01-17 04:18:05.112-0600

Based on your analysis, i guess changing:
if ((res < EL_BUF_SIZE - 1) && ((buf[res-1] == '\n') || (buf[res-2] == '\n'))) {
to:
if ((res < EL_BUF_SIZE - 1) && ((res >= 1 && buf[res-1] == '\n') || (res >= 2 && buf[res-2] == '\n'))) {

which should prevent the array 'underrun' situation for both situations where res = 1 or res = 2.


By: Diederik de Groot (dkdegroot) 2016-01-22 06:38:44.636-0600

@Badalian Vyacheslav: Could you recheck using the latest asterisk-11 / asterisk-13 revision to see if the issue has been fixed correctly. Please report back so this issue can be closed (or not).
By: Badalian Vyacheslav (slavon) 2016-01-22 08:29:39.431-0600

Thanks. But i can't now test :(
By: Corey Farrell (coreyfarrell) 2016-02-12 10:45:50.568-0600

A commit was done to fix this so I'm marking the ticket resolved.  If this is incorrect you can comment here to automatically reopen the ticket.