search

KhronosGroup / SPIRV-Reflect

SPIRV-Reflect is a lightweight library that provides a C/C++ reflection API for SPIR-V shader bytecode in Vulkan applications.
Apache License 2.0
672 stars 147 forks source link

Null pointer dereference #227

Closed coolkingcole closed 11 months ago

coolkingcole commented 11 months ago

Environment

Distributor ID: Debian
Description:    Debian GNU/Linux bookworm/sid

Version

I checked against the latest release as of 09/30/23 the current master branch at commit 764da20560fe518f399982fe5bc3232fd3fae76c .

Description

This AddressSanitizer output is indicating an OOB read of address 0x000000000004. This exception being on the zero page points to the root cause being a null pointer dereference. The cause of this is not properly checking the return of the FindNode() function that can return NULL. This null pointer is used as struct type SpvReflectPrvNode and it's members accessed as though it is a valid struct without validation. The provided POC file produces ASAN output for the stuct access in SPIRV-Reflect/blob/main/spirv_reflect.c at line 634. Other code sites that call FindNode() without checking the return may also result in a crash.

SPIRV-Reflect/blob/main/spirv_reflect.c:lines 632-L636

bool IsPointerToPointer(SpvReflectPrvParser* p_parser, uint32_t type_id) {
  SpvReflectPrvNode* ptr_node = FindNode(p_parser, type_id);
  if (ptr_node->op != SpvOpTypePointer) {
    return false;
  }

POC

./spirv-reflect ./POC_1

POC File

ASAN

AddressSanitizer:DEADLYSIGNAL
=================================================================
==320789==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000004 (pc 0x55555567c5a9 bp 0xffffffffffffffff sp 0x7fffffffbed0 T0)
==320789==The signal is caused by a READ memory access.
==320789==Hint: address points to the zero page.
    #0 0x55555567c5a9 in IsPointerToPointer /path/SPIRV-Reflect/spirv_reflect.c:634:20
    #1 0x55555567c5a9 in ParseDescriptorBlockVariableUsage /path/SPIRV-Reflect/spirv_reflect.c:2819:16
    #2 0x55555565d0dd in ParseDescriptorBlocks /path/SPIRV-Reflect/spirv_reflect.c:2865:16
    #3 0x55555565d0dd in CreateShaderModule /path/SPIRV-Reflect/spirv_reflect.c:4086:14
    #4 0x555555651659 in spv_reflect::ShaderModule::ShaderModule(unsigned long, void const*, unsigned int) /path/SPIRV-Reflect/spirv_reflect.h:1612:14
    #5 0x555555651659 in main /path/SPIRV-Reflect/main.cpp:135:31
    #6 0x7ffff7a461c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #7 0x7ffff7a46284 in __libc_start_main csu/../csu/libc-start.c:360:3
    #8 0x55555558c750 in _start (/path/SPIRV-Reflect/bin/spirv-reflect+0x38750) (BuildId: 9fde8772a1f81ca6c61bf2c2a5d43f7b9d38000d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /path/SPIRV-Reflect/spirv_reflect.c:634:20 in IsPointerToPointer
==320789==ABORTING

Other similar code sites not validated by POC file: https://github.com/KhronosGroup/SPIRV-Reflect/blob/764da20560fe518f399982fe5bc3232fd3fae76c/spirv_reflect.c#L640 https://github.com/KhronosGroup/SPIRV-Reflect/blob/764da20560fe518f399982fe5bc3232fd3fae76c/spirv_reflect.c#L634

This is a resubmission that was miscategorized as a security issue.

spencer-lunarg commented 11 months ago

we just recently added ASAN to our CI build https://github.com/KhronosGroup/SPIRV-Reflect/pull/223

I am curious how this is not getting caught/detected in that flow (haven't had time to look into, but wanted to note)

coolkingcole commented 11 months ago

You may not have a testcase that triggers this bug. I recommend making a libfuzzer harness. I can't recommend fuzzing directly on every commit as that would be a major slow down, but integration with OSS-fuzz is a good goal.