| Summary: | Heap use-after-free while using %clear inside a macro. | ||
|---|---|---|---|
| Product: | NASM | Reporter: | Marco <mvanotti> |
| Component: | Assembler | Assignee: | nobody <nobody> |
| Status: | OPEN --- | ||
| Severity: | normal | CC: | chang.seok.bae, gorcunov, hpa, mvanotti, nasm-bugs |
| Priority: | Medium | ||
| Version: | 2.16.xx | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Obtained from: | Built from git using configure | Generated by: | --- |
| Bug category: | Breaks existing code: | --- | |
This is an error for the same reason %unmacro in a running macro is. This obviously applies specifically to macros; clearing other things like defaliases are totally ok. Thanks for the reply, Peter. So a reasonable fix would be to disallow usage of %clear inside multi-line macros? Something like: * Disable %clear * Disable %clear all * Disable %clear macro I have never used context-local macros, so I am probably missing something there. I'm wondering if we shouldn't simply use the same teardown code as for %unmacro. There is already an invocation counter in the macro structure, so it isn't like we don't need to search the macro stack. Hi Peter, I have some time to give this a go. Here is a patch fixing this issue, and a few related smaller issues (for example, the %clear all never ever worked. It just loops endlessly on my computer). It still needs tests, but I have a couple of test cases that I can add, once I figure out how nasm tests work :) Please take a look when you have some time: https://github.com/netwide-assembler/nasm/commit/54b579b04b2a8c1dbf6a7acf1e26037a78e7a509 While looking at it, I noticed that there is some room for improvement in the handling of the %clear arguments. In particular, it seems to be possible to do something like %clear all all all all all all all all (etc), and something like %clear local defines global defines The documentation mentions that the context should be at the start (if present). With that in mind, the code can be simplified a bit. Like parsing first the context, then the rest of the parameters, and finally, making a single call to do_clear, checking for the mmacros expansion beforehand. Hi, I was wondering if there were any updates on this issue. Does the patch need any more work? |
It looks like if you put a %clear directive inside a macro, it will trigger a heap use-after-free. I am not sure what should happen when you do a %clear inside a macro, but probably not a use-after-free. See the following reproducer: ``` %macro baz 0 %clear %endmacro baz ``` Run with: ```shell $ nasm --pragma 'preproc sane_empty_expansion' -f elf64 -g -FDWARF -werror -o tmp.o file.asm ``` Compile nasm with address sanitizer: ```shell ./configure --enable-sanitizer ``` Address Sanitizer crash report (lines are based on commit e2ed7b7e125e03646d3be537b11d1b46b9c3f506): ```text ==721371==ERROR: AddressSanitizer: heap-use-after-free on address 0x610000001e48 at pc 0x5619f54196ed bp 0x7ffc966f4de0 sp 0x7ffc966f4dd0 READ of size 8 at 0x610000001e48 thread T0 #0 0x5619f54196ec in pp_tokline asm/preproc.c:7075 #1 0x5619f54196ec in pp_getline asm/preproc.c:7328 #2 0x5619f539363c in assemble_file asm/nasm.c:1722 #3 0x5619f5389d87 in main asm/nasm.c:717 #4 0x7fef1f8ae0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) #5 0x5619f538c57d in _start (/home/markov/nasm-fix-heap-overflow/nasm+0x2af57d) 0x610000001e48 is located 8 bytes inside of 192-byte region [0x610000001e40,0x610000001f00) freed by thread T0 here: #0 0x7fef204f37cf in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0x10d7cf) #1 0x5619f53eefa3 in free_mmacro_table asm/preproc.c:1054 #2 0x5619f54024de in do_directive asm/preproc.c:3938 #3 0x5619f541319e in pp_tokline asm/preproc.c:7264 #4 0x5619f541319e in pp_getline asm/preproc.c:7328 #5 0x5619f539363c in assemble_file asm/nasm.c:1722 #6 0x5619f5389d87 in main asm/nasm.c:717 #7 0x7fef1f8ae0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) previously allocated by thread T0 here: #0 0x7fef204f3dc6 in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10ddc6) #1 0x5619f5395a3b in nasm_calloc nasmlib/alloc.c:72 #2 0x5619f5395a95 in nasm_zalloc nasmlib/alloc.c:87 #3 0x5619f5402c7b in do_directive asm/preproc.c:4246 #4 0x5619f541319e in pp_tokline asm/preproc.c:7264 #5 0x5619f541319e in pp_getline asm/preproc.c:7328 #6 0x5619f539363c in assemble_file asm/nasm.c:1722 #7 0x5619f5389d87 in main asm/nasm.c:717 #8 0x7fef1f8ae0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) SUMMARY: AddressSanitizer: heap-use-after-free asm/preproc.c:7075 in pp_tokline Shadow bytes around the buggy address: 0x0c207fff8370: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c207fff8380: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x0c207fff8390: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c207fff83a0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x0c207fff83b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x0c207fff83c0: fa fa fa fa fa fa fa fa fd[fd]fd fd fd fd fd fd 0x0c207fff83d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c207fff83e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c207fff83f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c207fff8400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c207fff8410: 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 Shadow gap: cc ==721371==ABORTING ```