Skip to content

libshaderc_util: assert pos <= size() in string_piece::substr - #1584

Merged
dneto0 merged 1 commit into
google:mainfrom
BiswajeetRay7:fix-string-piece-substr-bounds-check
Jul 24, 2026
Merged

libshaderc_util: assert pos <= size() in string_piece::substr#1584
dneto0 merged 1 commit into
google:mainfrom
BiswajeetRay7:fix-string-piece-substr-bounds-check

Conversation

@BiswajeetRay7

Copy link
Copy Markdown
Contributor

Summary

string_piece::substr(pos, len = npos) currently only asserts
len == npos || pos + len <= size(). When len == npos (the default),
the assertion short-circuits and pos > size() is silently allowed,
producing a string_piece with begin_ > end_.

This invalid string_piece then cascades to unsound behavior in every
downstream accessor:

  • size() returns a wrapped value near SIZE_MAX due to
    signed-to-unsigned conversion of a negative ptrdiff_t.
  • empty() falsely returns false.
  • find_first_of / find_last_of iterate for the wrapped size,
    producing out-of-bounds reads.
  • operator<< passes a negative std::streamsize to
    std::ostream::write.
  • data() returns a dangling pointer past the end of the buffer.

Only lstrip / rstrip currently defend against inversion (lines 200
and 209).

Relation to prior hardening

Commit cb6f1ef ("Avoid invalid string_view iterators when parsing
#line directives") addressed one caller-side occurrence of this
class by tightening a starts_with check. This change addresses the
primitive itself, so that any future caller introducing the same
pattern is caught in debug builds.

Change

A single additional assert(pos <= size()) before the existing
assertion. No behavior change in release builds. No API change.

Reference

Reported via Google OSS VRP issue tracker #537914890.builds.

The existing assertion 'len == npos || pos + len <= size()' uses
short-circuit evaluation: when len == npos (the default), the second
clause is never evaluated, so pos > size() is silently allowed. This
produces a string_piece with begin_ > end_, which then causes size()
to wrap around near SIZE_MAX and cascades to out-of-bounds reads in
find_first_of, operator<<, and other accessors.

This follows the same class of hardening as commit cb6f1ef (Avoid
invalid string_view iterators when parsing #line directives), which
fixed one specific caller. This change addresses the primitive itself
so that any future caller is caught in debug builds.
@dj2
dj2 requested a review from dneto0 July 23, 2026 14:05
@dneto0
dneto0 enabled auto-merge (rebase) July 23, 2026 14:28

@dneto0 dneto0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you

@dneto0
dneto0 merged commit 40d725c into google:main Jul 24, 2026
24 checks passed
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.

3 participants