Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uninitialized value usage of localspluskinds in assemble.c's makecode function #119666

Closed
ammaraskar opened this issue May 28, 2024 · 7 comments
Closed
Assignees
Labels
type-bug An unexpected behavior, bug, or error type-crash A hard crash of the interpreter, possibly with a core dump type-security A security issue

Comments

@ammaraskar
Copy link
Member

ammaraskar commented May 28, 2024

Bug report

Bug description:

Recreator

./python -c "class i:[super for()in d]*[__class__*4for()in d]"
<string>:1: SyntaxWarning: invalid decimal literal
[1]    23793 segmentation fault  ./python -c "class i:[super for()in d]*[__class__*4for()in d]"

Details

This issue was found through the oss-fuzz compilation fuzzer. Here is the MSAN stack trace:

==691==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5661f67ca290 in get_localsplus_counts cpython3/Objects/codeobject.c:344:13
    #1 0x5661f67c95a7 in _PyCode_Validate cpython3/Objects/codeobject.c:433:5
    #2 0x5661f6a17be2 in makecode cpython3/Python/assemble.c:614:8
    #3 0x5661f6a17be2 in _PyAssemble_MakeCodeObject cpython3/Python/assemble.c:754:14
    #4 0x5661f612aa99 in optimize_and_assemble_code_unit cpython3/Python/compile.c:7655:10
    ...

 Uninitialized value was created by a heap allocation
    #0 0x5661f5b307b2 in __interceptor_malloc /src/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:1007:3
    #1 0x5661f675e32c in _PyBytes_FromSize cpython3/Objects/bytesobject.c:96:31
    #2 0x5661f675e00a in PyBytes_FromStringAndSize cpython3/Objects/bytesobject.c:129:27
    #3 0x5661f6a15d32 in makecode cpython3/Python/assemble.c:580:23
    #4 0x5661f6a15d32 in _PyAssemble_MakeCodeObject cpython3/Python/assemble.c:754:14
   ...

I haven't done any debugging yet but my hunch is that this code is hitting a path in compute_localsplus_info

compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus,

that ends up not setting the localspluskinds made here

cpython/Python/assemble.c

Lines 580 to 587 in f912e5a

localspluskinds = PyBytes_FromStringAndSize(NULL, nlocalsplus);
if (localspluskinds == NULL) {
goto error;
}
if (compute_localsplus_info(umd, nlocalsplus,
localsplusnames, localspluskinds) == ERROR) {
goto error;
}

and when this eventually gets to _PyCode_Validate it causes it to read uninitialized memory.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

@ammaraskar ammaraskar added the type-bug An unexpected behavior, bug, or error label May 28, 2024
@alex alex added type-security A security issue type-crash A hard crash of the interpreter, possibly with a core dump labels May 28, 2024
@alex
Copy link
Member

alex commented May 28, 2024

Marked as security and crash, since msan indicates UB which are all provisionally security issues.

@ammaraskar
Copy link
Member Author

cc @iritkatriel @markshannon as owners of assemble.c

@iritkatriel iritkatriel self-assigned this May 28, 2024
@iritkatriel
Copy link
Member

iritkatriel commented Jun 3, 2024

It seems that __class__ here is in u_varnames, u_cellvars and u_freevars.
There are two more args: .0 and super.

In optimize_and_assemble_code_unit it calculates that nlocalsplus is 4, which is incorrect. In compute_localsplus_info it populates 3 items in localsplus, and since it has size 4, this leaves entry 3 as NULL.

Question is whether this needs to be fixed in the compiler, or there is something here that should be rejected by the parser.

CC @carljm .

@iritkatriel
Copy link
Member

CC @JelleZijlstra

@JelleZijlstra
Copy link
Member

This should be fixed in the compiler; the code is legal. Here is a variation that runs fine in 3.11 but crashes (with a SystemError in this case) in 3.12:

class i: [super for _ in [1]] + [__class__ if False else 1 for _ in [2]]

@carljm
Copy link
Member

carljm commented Jun 3, 2024

Seems almost certainly related to comprehension inlining; I can take the investigation from here @iritkatriel if you want to assign to me.

@iritkatriel
Copy link
Member

Done.

@iritkatriel iritkatriel assigned carljm and unassigned iritkatriel Jun 3, 2024
carljm added a commit to carljm/cpython that referenced this issue Jun 9, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 10, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 10, 2024
@carljm carljm closed this as completed Jun 10, 2024
mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error type-crash A hard crash of the interpreter, possibly with a core dump type-security A security issue
Projects
None yet
Development

No branches or pull requests

5 participants