8388457: parse_integer: typo in hex prefix detection causes out-of-bounds read and wrong base for negative hex

Reviewed-by: dholmes, fbredberg
This commit is contained in:
David CARLIER 2026-07-21 22:23:28 +00:00 committed by David Holmes
parent 2be8e88ce7
commit 70868ac528
2 changed files with 69 additions and 2 deletions

View File

@ -124,7 +124,7 @@ static bool parse_integer(const char *s, char **endptr, T* result) {
T n = 0;
bool is_hex = (s[0] == '0' && (s[1] == 'x' || s[1] == 'X')) ||
(s[0] == '-' && s[1] == '0' && (s[2] == 'x' || s[3] == 'X'));
(s[0] == '-' && s[1] == '0' && (s[2] == 'x' || s[2] == 'X'));
char* remainder;
if (!parse_integer_impl<T>(s, &remainder, (is_hex ? 16 : 10), &n)) {

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2026, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022 SAP SE. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
@ -23,6 +23,7 @@
*/
#include "jvm_io.h"
#include "runtime/os.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/macros.hpp"
#include "utilities/ostream.hpp"
@ -156,3 +157,69 @@ TEST(ParseMemorySize, negatives_both) {
do_test_invalid_for_parse_arguments("100 M"); // parse_memory_size would see "100", parse_argument_memory_size would reject it
do_test_invalid_for_parse_arguments("100X"); // parse_memory_size would see "100", parse_argument_memory_size would reject it
}
// Hex prefix handling for negative numbers. Only signed types are covered here:
// parse_integer_impl() refuses a leading '-' for unsigned types before the base
// is ever used.
template <typename T>
static void test_negative_hex_prefix() {
T value = 17;
char* end = nullptr;
// Both spellings of the prefix must be recognized, just as "0x"/"0X" are for
// positive numbers.
ASSERT_TRUE(parse_integer("-0x10", &end, &value));
ASSERT_EQ(value, (T)-16);
EXPECT_STREQ(end, "");
ASSERT_TRUE(parse_integer("-0X10", &end, &value));
ASSERT_EQ(value, (T)-16);
EXPECT_STREQ(end, "");
ASSERT_TRUE(parse_integer("-0xff", &end, &value));
ASSERT_EQ(value, (T)-255);
ASSERT_TRUE(parse_integer("-0XFF", &end, &value));
ASSERT_EQ(value, (T)-255);
// A unit suffix still applies after a negative hex number.
ASSERT_TRUE(parse_integer("-0X10k", &end, &value));
ASSERT_EQ(value, (T)(-16 * K));
EXPECT_STREQ(end, "");
// "-0" is decimal; the character after the '0' decides, and here there is
// none. The value is zero either way, but see minus_zero_no_overread below.
ASSERT_TRUE(parse_integer("-0", &end, &value));
ASSERT_EQ(value, (T)0);
EXPECT_STREQ(end, "");
// Not a hex prefix: 'f' is not 'x'/'X', so this is decimal "-0" with "fX"
// left over, not hex "-0f".
value = 17;
ASSERT_TRUE(parse_integer("-0fX", &end, &value));
ASSERT_EQ(value, (T)0);
EXPECT_STREQ(end, "fX");
ASSERT_FALSE(parse_integer("-0fX", &value));
}
TEST(ParseMemorySize, negative_hex_prefix) {
test_negative_hex_prefix<int64_t>();
test_negative_hex_prefix<int32_t>();
}
// "-0" holds just two characters plus the terminating NUL, so hex prefix
// detection must stop at the NUL rather than read the character beyond it.
// The string is copied into a tightly sized allocation so that an over-read
// is caught when running under a memory checker such as ASan.
TEST(ParseMemorySize, minus_zero_no_overread) {
char* s = os::strdup("-0", mtTest);
int64_t value = 17;
char* end = nullptr;
ASSERT_TRUE(parse_integer(s, &end, &value));
ASSERT_EQ(value, (int64_t)0);
EXPECT_STREQ(end, "");
os::free(s);
}