Skip to content

gh-146525: Add ndim validation to PyBuffer_ToContiguous for defense-in-depth#146523

Closed
l3tchupkt wants to merge 1 commit intopython:mainfrom
l3tchupkt:gh-ndim-validation-hardening
Closed

gh-146525: Add ndim validation to PyBuffer_ToContiguous for defense-in-depth#146523
l3tchupkt wants to merge 1 commit intopython:mainfrom
l3tchupkt:gh-ndim-validation-hardening

Conversation

@l3tchupkt
Copy link
Copy Markdown

@l3tchupkt l3tchupkt commented Mar 27, 2026

Summary

Add validation of the ndim parameter in PyBuffer_ToContiguous() to prevent potential integer overflow in memory allocation calculations. This is a defense-in-depth hardening measure.

Fixes #146525

Problem

The allocation in PyBuffer_ToContiguous():

fb = PyMem_Malloc(sizeof *fb + 3 * src->ndim * (sizeof *fb->array));

Could theoretically overflow if ndim is excessively large. While Python-level code already enforces PyBUF_MAX_NDIM (64), C extensions implementing custom getbufferproc could potentially provide invalid ndim values.

Solution

Add explicit validation before allocation to ensure ndim is within the valid range (0 to PyBUF_MAX_NDIM).

Changes

Objects/memoryobject.c:

  • Added runtime validation in PyBuffer_ToContiguous() to check ndim is within valid range (0-64)
  • Added assertion in buffer_to_contiguous() for consistency

Lib/test/test_memoryview.py:

  • Added test_ndim_limit() test case to verify:
    • ndim > 64 raises ValueError
    • Negative ndim is rejected

Misc/NEWS.d/next/C_API/2026-03-27-00-00-00.gh-146525.abc123.rst:

  • Added NEWS entry documenting the change

Testing

All existing tests pass: python -m test test_memoryview (140 tests OK)
New validation test passes
Verified the fix correctly rejects invalid ndim values

Security Classification

This is a hardening improvement, not a fix for an actively exploitable vulnerability.

No CVE is warranted because:

  • The overflow would require ndim > ~3.8×10^17 on 64-bit systems, which is not practically achievable
  • Python-level code already enforces PyBUF_MAX_NDIM (64)
  • Would require a malicious C extension providing impossible ndim values
  • This adds defense-in-depth validation for code quality and robustness

Why This Matters

  1. Defense-in-depth: Adds an extra layer of protection against malformed C extensions
  2. Code Quality: Makes assumptions explicit through validation
  3. Consistency: Aligns C-level checks with Python-level enforcement of PyBUF_MAX_NDIM
  4. Documentation: Clarifies the valid range for ndim in the C API

Author: Lakshmikanthan K (badassletchu@gmail.com)

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Mar 27, 2026

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Mar 27, 2026

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@l3tchupkt l3tchupkt force-pushed the gh-ndim-validation-hardening branch from ea5336e to f651393 Compare March 27, 2026 15:22
@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Mar 27, 2026

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@l3tchupkt l3tchupkt changed the title Add ndim validation to PyBuffer_ToContiguous for defense-in-depth gh-146525: Add ndim validation to PyBuffer_ToContiguous for defense-in-depth Mar 27, 2026
@l3tchupkt l3tchupkt force-pushed the gh-ndim-validation-hardening branch 3 times, most recently from e01f2f6 to 8215d60 Compare March 27, 2026 16:01
Copy link
Copy Markdown
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

I'm not convinced by this change. cc @vstinner

Comment on lines +1063 to +1066
/* Validate ndim to prevent potential integer overflow in allocation.
* While Python-level code enforces PyBUF_MAX_NDIM, C extensions could
* potentially provide invalid values. This is a defense-in-depth check. */
if (src->ndim < 0 || src->ndim > PyBUF_MAX_NDIM) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think it's really necessary:

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Mar 27, 2026

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@l3tchupkt l3tchupkt closed this Mar 28, 2026
@l3tchupkt l3tchupkt force-pushed the gh-ndim-validation-hardening branch from 8215d60 to f042fa6 Compare March 28, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ndim validation to PyBuffer_ToContiguous for defense-in-depth

2 participants