Heap buffer overflow read in weight packing when filter shape dims[1..3] mismatch convolution kernel params
- Dominant language
- C
- Stars
- 2.5k
- Forks
- 560
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 163
Description
## Summary
`xnn_define_convolution_2d` validates only `filter_value->shape.dim[0] == group_output_channels * groups` (convolution-2d.c:1298). It does not validate `dim[1] == kernel_height`, `dim[2] == kernel_width`, or `dim[3] == group_input_channels`. When a filter tensor is declared with dims[1..3] smaller than the convolution's kernel parameters, weight packing (`xnn_pack_f32_conv_goki_w`) reads `nc * ks * kc` elements from the filter buffer — where `ks = kernel_height * kernel_width` comes from the **convolution parameters**, not the filter's declared shape — causing a heap out-of-bounds read during `xnn_create_runtime_v2`.
`xnn_define_deconvolution_2d` and `xnn_define_depthwise_convolution_2d` have **no filter shape validation at all** (not even dim[0]).
## Vulnerability Details
**Bug class:** Heap out-of-bounds read (CWE-125)
**Affected functions:** `xnn_define_convolution_2d`, `xnn_define_deconvolution_2d`, `xnn_define_depthwise_convolution_2d` (subgraph API)
**Sink:** `xnn_pack_f32_conv_goki_w` at `src/reference/packing.cc:3553` (`std::copy_n`)
**Trigger:** `xnn_create_runtime_v2` / `xnn_create_runtime_v4` (during weight packing, before any inference)
## Proof of Concept
Filter tensor declared as shape `{32, 3, 3, 3}` = 864 floats. Convolution defined with `kernel_height=5, kernel_width=5, group_input_channels=3, group_output_channels=32, groups=1`. The dim[0] check passes (32 == 32 * 1). Weight packing reads `32 * 25 * 3 = 2400` floats from the 864-float buffer → **1536-float (6144-byte) heap OOB read**.
```c
// Minimal repro — full source in attached poc_s1.c
xnn_subgraph_t subgraph;
xnn_create_subgraph(2, 0, &subgraph);
size_t filter_dims[4] = {32, 3, 3, 3}; // 864 floats
float *filter_data = malloc(864 * sizeof(float));
uint32_t filter_id;
xnn_define_tensor_value(subgraph, xnn_datatype_fp32, 4, filter_dims,
filter_data, XNN_INVALID_VALUE_ID, 0, &filter_id);
// ... define input, bias, output tensors ...
xnn_define_convolution_2d(subgraph,
/*padding*/ 0, 0, 0, 0,
/*kernel*/ 5, 5, // <-- mismatch: filter says 3x3, conv says 5x5
/*subsampling*/ 1, 1,
/*dilation*/ 1, 1,
/*groups*/ 1,
/*group_input_channels*/ 3,
/*group_output_channels*/ 32,
/*output_min*/ -1e30f, /*output_max*/ 1e30f,
input_id, filter_id, bias_id, output_id, 0);
xnn_runtime_t runtime;
xnn_create_runtime_v2(subgraph, NULL, 0, &runtime); // <-- crashes here
```
### ASan output (HEAD 6c48650, arm64, clang ASan)
```
==51790==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61f000000e90
READ of size 4 at 0x61f000000e90 thread T0
#0 __asan_memmove+0x2a4
#1 xnn_pack_f32_conv_goki_w packing.cc:3553
#2 create_igemm convolution-nhwc.c:452
#3 create_convolution2d_nhwc convolution-nhwc.c:806
#4 xnn_create_convolution2d_nhwc_pf32 convolution-nhwc.c:2559
#5 create_convolution_operator convolution-2d.c:261
#6 xnn_create_runtime_v4 runtime.c:727
#7 xnn_create_runtime_v2 runtime.c:219
#8 main poc_s1.c:143
0x61f000000e90 is located 144 bytes after 3456-byte region [0x61f000000080,0x61f000000e00)
allocated by thread T0 here:
#0 malloc+0x94
#1 main poc_s1.c:52
SUMMARY: AddressSanitizer: heap-buffer-overflow in xnn_pack_f32_conv_goki_w packing.cc:3553
```
Reproduced twice, deterministic. Full PoC source + build commands: see attached `poc_s1.c`.
## Impact
A heap out-of-bounds read occurs during model loading (`xnn_create_runtime_v2`), before any inference runs. The over-read data flows into packed weights and influences inference output. This is a memory-safety bug reachable via the public subgraph C API when loading a model with inconsistent filter shape and convolution parameters.
The packed-weights destination buffer is sized from the convolution parameters (not the filter shape), so no out-of-bounds **write** is reachable — this is read-only (CWE-125, not CWE-787).
## Recommended Fix
Add filter shape validation in `xnn_define_convolution_2d` after the existing dim[0] check (convolution-2d.c:1298):
```c
if (filter_value->shape.num_dims != 4 ||
filter_value->shape.dim[1] != kernel_height ||
filter_value->shape.dim[2] != kernel_width ||
filter_value->shape.dim[3] != group_input_channels) {
xnn_log_error(
"failed to define %s operator with filter shape ..."
": filter must be [groups*group_output_channels, kernel_height, "
"kernel_width, group_input_channels]",
...);
return xnn_status_invalid_parameter;
}
```
Apply the same pattern to `xnn_define_deconvolution_2d` and `xnn_define_depthwise_convolution_2d` (which currently have no filter shape validation at all).
## Build + Reproduce
```bash
cd
mkdir -p build-asan
cmake -S . -B build-asan \
-DXNNPACK_BUILD_TESTS=OFF -DXNNPACK_BUILD_BENCHMARKS=OFF \
-DXNNPACK_ENABLE_ASSEMBLY=OFF \
-DCMAKE_C_FLAGS="-fsanitize=address -g -O1" \
-DCMAKE_CXX_FLAGS="-fsanitize=address -g -O1"
cmake --build build-asan --target XNNPACK -j 8
clang -fsanitize=address -g -O1 -I include poc_s1.c -o poc_s1 \
build-asan/libXNNPACK.a build-asan/libxnnpack-microkernels-prod.a \
build-asan/pthreadpool/libpthreadpool.a build-asan/cpuinfo/libcpuinfo.a \
build-asan/kleidiai/libkleidiai.a -lpthread -lm -lc++ -fsanitize=address
./poc_s1
```
Contributor guide
Research direction
Start in convolution-2d.c at the filter validation in xnn_define_convolution_2d, then inspect xnn_define_deconvolution_2d and xnn_define_depthwise_convolution_2d and the packing sink at src/reference/packing.cc:3553. Use the attached poc_s1.c with the ASan build to reproduce the runtime-creation read. Done means inconsistent filter shapes are rejected before weight packing for all three APIs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- api, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100