Skip to content

Fix integer overflow in tensor dimensions and PPM parser#896

Open
sharadboni wants to merge 2 commits intogoogle:devfrom
sharadboni:fix-tensor-ppm-overflow
Open

Fix integer overflow in tensor dimensions and PPM parser#896
sharadboni wants to merge 2 commits intogoogle:devfrom
sharadboni:fix-tensor-ppm-overflow

Conversation

@sharadboni
Copy link
Copy Markdown

Summary

  • Tensor dimension overflow (util/basics.h): Extents2D::Area() computes rows * cols without checking for size_t overflow. When tensor dimensions are deserialized from a model file via MatPtr::VisitFields(), attacker-controlled rows and cols values can silently overflow, causing undersized allocations and subsequent heap buffer overflows. Added an overflow check that aborts if cols > SIZE_MAX / rows.

  • PPM image size overflow (paligemma/image.cc): ReadPPM() computes data_size = width * height * 3 without overflow protection. A crafted PPM file with large width/height values can overflow this computation, leading to an undersized data_ vector and out-of-bounds writes during pixel parsing. Added a two-step overflow check before the multiplication.

  • PPM integer parsing overflow (paligemma/image.cc): ParseUnsigned() accumulates decimal digits into a size_t without detecting overflow, allowing silently wrapped values to bypass subsequent size checks. Added overflow detection that returns nullptr (parse failure) when the accumulated value would exceed SIZE_MAX.

Test plan

  • Verify existing tests still pass (bazel test //...)
  • Confirm PPM files with normal dimensions load correctly
  • Confirm malformed PPM with overflow-inducing dimensions is rejected gracefully
  • Confirm model files with overflow-inducing tensor dimensions trigger abort

@sharadboni
Copy link
Copy Markdown
Author

@jan-wassenberg Could you review this security fix? It adds overflow checks for tensor dimensions in Extents2D::Area() and fixes integer overflow in the PPM image parser that can be triggered via crafted model/image files.

Copy link
Copy Markdown
Member

@jan-wassenberg jan-wassenberg left a comment

Choose a reason for hiding this comment

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

Thanks, this seems reasonable to rule out. We're only able to land patches targeting the dev branch, would you mind updating the PR?
Also one code suggestion.

Comment thread paligemma/image.cc Outdated
- util/basics.h: Add overflow check in Extents2D::Area() before
  computing rows*cols. Malicious model files with large dimension
  values could cause a silent size_t overflow, leading to undersized
  allocations and subsequent heap buffer overflows.

- paligemma/image.cc: Add overflow check for width*height*3 in
  ReadPPM(). A crafted PPM file with large dimensions could overflow
  the data_size computation, resulting in an undersized buffer and
  out-of-bounds writes.

- paligemma/image.cc: Add overflow detection in ParseUnsigned() to
  reject values that would overflow size_t during decimal parsing.
@sharadboni sharadboni force-pushed the fix-tensor-ppm-overflow branch from 8bf2a7b to c4d14db Compare April 16, 2026 16:02
@sharadboni sharadboni changed the base branch from main to dev April 16, 2026 16:02
Copy link
Copy Markdown
Member

@jan-wassenberg jan-wassenberg left a comment

Choose a reason for hiding this comment

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

Thanks for updating! (FYI HWY_ABORT does not return, but we can land as-is)

@jan-wassenberg jan-wassenberg added the copybara-import Trigger Copybara for merging pull requests label Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copybara-import Trigger Copybara for merging pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants