RFR: 8368097: [asan] heap-buffer-overflow reported in ClassFileParser::skip_over_field_signature
David Holmes
dholmes at openjdk.org
Thu Oct 2 04:46:45 UTC 2025
On Fri, 26 Sep 2025 12:59:56 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
> Hi,
>
> `skip_over_field_name` may produce a pointer which is exactly one `char` of bounds, which is the dereferenced by `skip_over_field_signature` when it looks for a semi-colon. This causes an out-of-bounds read, which ASAN caught. The fix is to check whether it's OK to dereference `p` or not.
>
> We keep the semantics the same other than that, so `skip_over_field_signature` and `skip_over_field_name` can both return a pointer which is one past the valid memory range. Creating such a pointer is explicitly not UB, but dereferencing it is.
Fix looks good - thanks. My own view is that `(p - signature) > 1` was a mal-formed attempt at checking at least one more character was present.
src/hotspot/share/classfile/classFileParser.cpp line 4688:
> 4686: // The next character better be a semicolon
> 4687: if (p != nullptr && // Parse of field name succeeded.
> 4688: p - signature < static_cast<int>(length) && // There is at least one character left to parse.
Suggestion:
if (p != nullptr && // Parse of field name succeeded.
p - signature < static_cast<int>(length) && // There is at least one character left to parse.
I'm not a fan of this kind of alignment but lets at least not add unnecessary whitespace. :)
-------------
Marked as reviewed by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/27528#pullrequestreview-3292254739
PR Review Comment: https://git.openjdk.org/jdk/pull/27528#discussion_r2396815363
More information about the hotspot-runtime-dev
mailing list