Skip to content

Conversation

hongjinghao
Copy link

paramlen has heap memory of length nparam+1. The value of variable i may be greater than nparam+1, causing heap memory overflow. Therefore, i and nparam+1 needs to be determined in the loop.
Fix:https://bugzilla.nasm.us/show_bug.cgi?id=3392857#c1

@sezero
Copy link
Contributor

sezero commented Sep 5, 2023

Patch is utterly broken because:

(1) wrong parenthesing:

asm/preproc.c: In function ‘expand_mmacro’:
asm/preproc.c:6820: warning: assignment makes pointer from integer without a cast

... and:
(2) it allows out-of-bounds read

... and still segfaults, of course:

$ valgrind ./nasm -f elf64 ../nasm-crash -o 1.o
==5002== Memcheck, a memory error detector
==5002== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==5002== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==5002== Command: ./nasm -f elf64 ../nasm-crash -o 1.o
==5002== 
../nasm-crash:2: error: parser: instruction expected
../nasm-crash:5: error: label or instruction expected at start of line
../nasm-crash:6: error: parser: instruction expected
../nasm-crash:8: error: label or instruction expected at start of line
../nasm-crash:9: error: parser: instruction expected
../nasm-crash:12: warning: unterminated %[...] construct (missing `]') [-w+pp-open-brackets]
../nasm-crash:12: error: `%0': not in a macro call
../nasm-crash:12: error: `%1': not in a macro call
../nasm-crash:12: error: `%2': not in a macro call
../nasm-crash:12: error: `%3': not in a macro call
../nasm-crash:12: error: `%4': not in a macro call
../nasm-crash:12: error: `%5': not in a macro call
../nasm-crash:12: warning: label alone on a line without a colon might be in error [-w+label-orphan]
../nasm-crash:13: error: `%ENDMACRO': not defining a macro
../nasm-crash:14: error: parser: instruction expected
../nasm-crash:15: error: parser: instruction expected
../nasm-crash:28: error: parser: instruction expected
../nasm-crash:29: error: parser: instruction expected
==5002== Invalid read of size 4
==5002==    at 0x8061168: pp_tokline (preproc.c:673)
==5002==    by 0x8063D6C: pp_getline (preproc.c:7823)
==5002==    by 0x804A710: assemble_file (nasm.c:1719)
==5002==    by 0x804C4CC: main (nasm.c:716)
==5002==  Address 0x5 is not stack'd, malloc'd or (recently) free'd
==5002== 
==5002== 
==5002== Process terminating with default action of signal 11 (SIGSEGV)
==5002==  Access not within mapped region at address 0x5
==5002==    at 0x8061168: pp_tokline (preproc.c:673)
==5002==    by 0x8063D6C: pp_getline (preproc.c:7823)
==5002==    by 0x804A710: assemble_file (nasm.c:1719)
==5002==    by 0x804C4CC: main (nasm.c:716)

Correct one should be like:

diff --git a/asm/preproc.c b/asm/preproc.c
index ac42131e..24f167b9 100644
--- a/asm/preproc.c
+++ b/asm/preproc.c
@@ -6817,7 +6817,7 @@ static int expand_mmacro(Token * tline)
      */
     nasm_newn(paramlen, nparam+1);
 
-    for (i = 1; (t = params[i]); i++) {
+    for (i = 1; i < nparam+1 && (t = params[i]); i++) {
         bool braced = false;
         int brace = 0;
         int white = 0;

paramlen has heap memory of length nparam+1. The value of variable i may be greater than nparam+1, causing heap memory overflow. Therefore,  i and nparam+1 needs to be determined in the loop.
fix:https://bugzilla.nasm.us/show_bug.cgi?id=3392857#c1
@hpax
Copy link
Member

hpax commented Oct 6, 2025

This code has changed a lot. Is this still a problem?

@hpax hpax closed this in e39b856 Oct 7, 2025
@hpax
Copy link
Member

hpax commented Oct 7, 2025

Merged.

if (q)
if (q){
free(q);
q = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change to alloc.c is meaningless

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.

3 participants