Skip to content

Fix null pointer dereference on SPIR-V assembly failure - #1598

Open
esoalba wants to merge 2 commits into
google:mainfrom
esoalba:fix/assembly-result-null-deref
Open

esoalba wants to merge 2 commits into
google:mainfrom
esoalba:fix/assembly-result-null-deref

Conversation

@esoalba

@esoalba esoalba commented Sep 10, 2026

Copy link
Copy Markdown

Fixes #1597

shaderc_assemble_into_spv() leaves output_data_ null when assembly fails, but GetBytes() unconditionally dereferences it, crashing glslc, the C API (shaderc_result_get_bytes), and the C++ API (cbegin()/cend()) on any invalid .spvasm input.

This adds a null check before the dereference. Verified: invalid assembly now errors gracefully (exit 1 with an assembly error message) instead of segfaulting; valid assembly still compiles; C API returns nullptr on failure.

Unit test added (3cbbe7a): AssembleStringTest.FailedAssemblyGetBytesIsNull asserts that a failed assembly result has null bytes and zero length. Without the fix the test crashes with a null pointer dereference (SIGSEGV); with the fix it passes. Full test suite: 151 tests in shaderc_test, 126 in shaderc_cpp_test, all passing.

Reported via Google OSS VRP (g.co/vulnz).

@dneto0 dneto0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should get a unit test also.

dneto0 and others added 2 commits September 11, 2026 18:09
shaderc_assemble_into_spv() leaves output_data_ null when assembly
fails, but GetBytes() unconditionally dereferences it, crashing glslc,
the C API (shaderc_result_get_bytes), and the C++ API (cbegin/cend)
on any invalid .spvasm input.

Check output_data_ before dereferencing.
@dneto0
dneto0 force-pushed the fix/assembly-result-null-deref branch from 38726c0 to 9495460 Compare September 11, 2026 22:35
@dneto0

dneto0 commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

This should get a unit test also.

I've added unit tests. And rebased on top of #1603

@esoalba

esoalba commented Sep 11, 2026

Copy link
Copy Markdown
Author

Thanks @dneto0 — appreciate the review, and for taking the fix further. Hardening all the result getters against a null result object plus the NullResultObj and GetBytes* tests is a more complete fix than my original change, and the rebase on #1603 is appreciated.
On the test request: I had a complementary end-to-end regression test ready to push — shaderc_assemble_into_spv on invalid input, then asserting shaderc_result_get_bytes() returns nullptr on the failed result (verified it SIGSEGVs without the null check). I held off since your tests already cover the null-safety at the unit level. Happy to add it as an integration-level check if you think it's worth having; otherwise I'll leave the branch as-is.

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.

Null pointer dereference in shaderc SPIR-V assembly handling crashes glslc and API consumers

2 participants