Skip to content

Fix missing bounds checks in subgraph API functions#9845

Open
mohammadmseet-hue wants to merge 1 commit intogoogle:masterfrom
mohammadmseet-hue:fix/missing-bounds-checks-batch2
Open

Fix missing bounds checks in subgraph API functions#9845
mohammadmseet-hue wants to merge 1 commit intogoogle:masterfrom
mohammadmseet-hue:fix/missing-bounds-checks-batch2

Conversation

@mohammadmseet-hue
Copy link
Copy Markdown
Contributor

Summary

Multiple public API functions in ynnpack accept user-controlled count/axis parameters
that are used as loop bounds or array indices without bounds checking, causing stack
buffer overflows:

  • xnn_define_static_expand_dims (subgraph.cc): num_new_axes used as loop bound to write into int32_t ynn_axes[XNN_MAX_TENSOR_DIMS] (size 6). ASAN-confirmed stack-buffer-overflow with num_new_axes > 6.

  • xnn_define_static_reduce (subgraph.cc): num_reduction_axes used as loop bound to write into int64_t signed_reduction_axes[XNN_MAX_TENSOR_DIMS] (size 6). Same pattern.

  • ynn_define_broadcast (broadcast.cc): Missing validate_axis call. Out-of-range axes passed directly to axis_to_slinky_dim, producing negative results used as bitset<10> indices.

  • ynn_define_broadcast_like (broadcast_like.cc): Same missing validation.

  • ynn_define_reduce (reduce.cc): Same missing validation.

  • ynn_define_fuse_dims (copy.cc): Off-by-one — axes[i] is validated but axes[i]+1 is passed to axis_to_slinky_dim. When axes[i] = rank-1, axes[i]+1 = rank produces index -1.

Fixes

  • Add num_new_axes > XNN_MAX_TENSOR_DIMS / num_reduction_axes > XNN_MAX_TENSOR_DIMS bounds checks before stack array writes.
  • Add validate_axis calls before axis_to_slinky_dim in broadcast, broadcast_like, and reduce.
  • Add validate_axis for axes[i]+1 in fuse_dims.

ASAN trace (xnn_define_static_expand_dims, before fix)

==12==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f69d2e1a0d8
WRITE of size 4 at 0x7f69d2e1a0d8 thread T0
    #0 in xnn_define_static_expand_dims
  [64, 88) 'ynn_axes' (line 774) <== Memory access at offset 88 overflows this variable

Test plan

  • All 5 PoC tests pass with ASAN (no crashes after fix)
  • bazel test //ynnpack/subgraph/test:oob_write_poc --copt=-fsanitize=address --linkopt=-fsanitize=address passes

Add bounds validation for user-controlled parameters that were used as
loop bounds or array indices without checking:

- xnn_define_static_expand_dims: num_new_axes was used to write into
  int32_t ynn_axes[XNN_MAX_TENSOR_DIMS] without bounds check. Add
  check that num_new_axes <= XNN_MAX_TENSOR_DIMS.

- xnn_define_static_reduce: num_reduction_axes was used to write into
  int64_t signed_reduction_axes[XNN_MAX_TENSOR_DIMS] without bounds
  check. Add check that num_reduction_axes <= XNN_MAX_TENSOR_DIMS.

- ynn_define_broadcast: axes were passed to axis_to_slinky_dim without
  validate_axis, producing out-of-range bitset indices. Add
  validate_axis call.

- ynn_define_broadcast_like: same missing validate_axis. Add it.

- ynn_define_reduce: same missing validate_axis. Add it.

- ynn_define_fuse_dims: axes[i]+1 was passed to axis_to_slinky_dim but
  only axes[i] was validated. When axes[i]=rank-1, axes[i]+1=rank
  produces an invalid index. Add validate_axis for axes[i]+1.
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.

1 participant