[Nasm-bugs] [Bug 3392793] New: Stack use-after-scope in expand_mmac_params.

[noreply-nasm at dev.nasm.us]

https://bugzilla.nasm.us/show_bug.cgi?id=3392793

            Bug ID: 3392793
           Summary: Stack use-after-scope in expand_mmac_params.
           Product: NASM
           Version: 2.16 (development)
          Hardware: PC
                OS: Linux
            Status: OPEN
          Severity: normal
          Priority: Medium
         Component: Assembler
          Assignee: nobody at nasm.us
          Reporter: mvanotti at protonmail.com
                CC: chang.seok.bae at intel.com, gorcunov at gmail.com,
                    hpa at zytor.com, nasm-bugs at nasm.us
     Obtained from: Built from git using configure

Created attachment 411843
  --> https://bugzilla.nasm.us/attachment.cgi?id=411843&action=edit
Reproducer Test Case

The following test case triggers a stack use-after-scope in nasm.

```
%[%f
```

nasm compiled with:

`./configure --enable-sanitizer` and run with:

```
$ ASAN_OPTIONS="detect_leaks=0:detect_stack_use_after_return=1" ./nasm -felf64
-o /tmp/aaaa test.asm
```


Output:
```
user at sambayon:~/nasm$
ASAN_OPTIONS="detect_leaks=0:detect_stack_use_after_return=1" ./nasm -felf64 -o
/tmp/aaaa test.asm 
test.asm:1: warning: unterminated %[ construct [-w+other]
test.asm:1: error: `%1': not in a macro call
=================================================================
==47728==ERROR: AddressSanitizer: stack-use-after-return on address
0x7f24bae04fe0 at pc 0x55d8d42afcc2 bp 0x7ffe10bc5d70 sp 0x7ffe10bc5d68
WRITE of size 8 at 0x7f24bae04fe0 thread T0
    #0 0x55d8d42afcc1 in expand_mmac_params
/home/mvanotti/nasm/asm/preproc.c:5373:11
    #1 0x55d8d4289e74 in pp_tokline /home/user/nasm/asm/preproc.c:7258:21
    #2 0x55d8d4286089 in pp_getline /home/user/nasm/asm/preproc.c:7328:17
    #3 0x55d8d420d1cd in assemble_file /home/user/nasm/asm/nasm.c:1722:24
    #4 0x55d8d420aa21 in main /home/user/nasm/asm/nasm.c:717:9
    #5 0x7f24bd4890b2 in __libc_start_main
/build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #6 0x55d8d415746d in _start (/home/user/nasm/nasm+0x24146d) (BuildId:
3fc13de32457a8981b73bda01728cd257f86782c)

Address 0x7f24bae04fe0 is located in stack of thread T0 at offset 32 in frame
    #0 0x55d8d42ac50f in dup_tlist /home/user/nasm/asm/preproc.c:891

  This frame has 1 object(s):
    [32, 40) 'newlist' (line 892) <== Memory access at offset 32 is inside this
variable
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-use-after-return
/home/mvanotti/nasm/asm/preproc.c:5373:11 in expand_mmac_params
Shadow bytes around the buggy address:
  0x0fe5175b89a0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0fe5175b89b0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0fe5175b89c0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0fe5175b89d0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0fe5175b89e0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
=>0x0fe5175b89f0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5[f5]f5 f5 f5
  0x0fe5175b8a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fe5175b8a10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fe5175b8a20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fe5175b8a30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fe5175b8a40: 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
==47728==ABORTING
```

The issue seems to be that in expand_mmac_params, the case for TOKEN_INDIRECT
does not handle well failures:

```c
        case TOKEN_INDIRECT:
        {
            Token *tt;

            tt = tokenize(tok_text(t));
            tt = expand_mmac_params(tt);
            tt = expand_smacro(tt);
            /* Why dup_tlist() here? We should own tt... */
            dup_tlist(tt, &tail);
            text = NULL;
            change = true;
            break;
        }
```


if `tt` is NULL, `dup_tlist` will make `tail` point to garbage:

```
/*
 * Duplicate a linked list of tokens.
 */
static Token *dup_tlist(const Token *list, Token ***tailp)
{
    Token *newlist = NULL;
    Token **tailpp = &newlist;
    const Token *t;

    list_for_each(t, list) {
        Token *nt;
        *tailpp = nt = dup_Token(NULL, t);
        tailpp = &nt->next;
    }

    if (tailp) {
        **tailp = newlist;
        *tailp = tailpp;
    }

    return newlist;
}
```

Maybe it would be useful to make sure that list is not null in dup_tlist (add
an assert?) and check the possible error conditions in expand_mmac_params.
Something like:

```
            if (tt == NULL) {
              text = NULL;
              change = false;
              break;
            }
```

-- 
You are receiving this mail because:
You are watching all bug changes.
You are on the CC list for the bug.