Skip to content

CDRIVER-4662 Check for Decimal128 exponent overflow #1349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jul 24, 2023
Merged

CDRIVER-4662 Check for Decimal128 exponent overflow #1349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0331a34
add spec tests
joshbsiegel Jul 13, 2023
d2a6642
check for exponent too large, tests
joshbsiegel Jul 19, 2023
60824e3
fix test name
joshbsiegel Jul 19, 2023
5213f41
int64 min max tests
joshbsiegel Jul 21, 2023
ab802e4
add int32_t conversion
joshbsiegel Jul 21, 2023
661d578
modfiy from PRId64 to SCNd64
joshbsiegel Jul 21, 2023
43d6a48
add print statement
joshbsiegel Jul 21, 2023
928a132
more print statements
joshbsiegel Jul 21, 2023
43e6094
print if statement conditionals and rework ifdef
joshbsiegel Jul 21, 2023
7c77480
int64 conversions on int32 min and max
joshbsiegel Jul 21, 2023
6b5eb17
min and max explicit conversions
joshbsiegel Jul 21, 2023
8fc7427
cleaning up
joshbsiegel Jul 21, 2023
fd02d95
add empty lines back in
joshbsiegel Jul 21, 2023
0de9502
revert exponent type, use existing range checker
joshbsiegel Jul 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/libbson/src/bson/bson-decimal128.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,14 +581,18 @@ bson_decimal128_from_string_w_len (const char *string, /* IN */
#else
#define SSCANF sscanf
#endif
int read_exponent = SSCANF (++str_read, "%d%n", &exponent, &nread);
int64_t temp_exponent = 0;
int read_exponent =
SSCANF (++str_read, "%" SCNd64 "%n", &temp_exponent, &nread);
str_read += nread;

if (!read_exponent || nread == 0) {
if (!read_exponent || nread == 0 ||
!bson_in_range_int32_t_signed (temp_exponent)) {
BSON_DECIMAL128_SET_NAN (*dec);
return false;
}

exponent = (int32_t) temp_exponent;
#undef SSCANF
}

Expand Down
4 changes: 2 additions & 2 deletions src/libbson/src/bson/bson-json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1124,9 +1124,9 @@ _bson_json_read_string (bson_json_reader_t *reader, /* IN */
} break;
case BSON_JSON_LF_DECIMAL128: {
bson_decimal128_t decimal128;
bson_decimal128_from_string (val_w_null, &decimal128);

if (bson->read_state == BSON_JSON_IN_BSON_TYPE) {
if (bson_decimal128_from_string (val_w_null, &decimal128) &&
bson->read_state == BSON_JSON_IN_BSON_TYPE) {
bson->bson_type_data.v_decimal128.value = decimal128;
} else {
goto BAD_PARSE;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On failure, BAD_PARSE will set the error message as

Invalid input string "0E+xxx", looking for DECIMAL128

I think this message is fine as is, but potentially could be more helpful to the user with some changes. I believe it would involve adding some return information from bson_decimal128_from_string as to why the parsing fails (because it may not have to do with an overflowing exponent). Open to thoughts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it may help to determine the cause of the error with a new function: bson_decimal128_from_string_w_error. But I think that is separate from this bug fix and could be considered in a future change.

Expand Down
24 changes: 24 additions & 0 deletions src/libbson/tests/json/bson_corpus/decimal128-1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,30 @@
"canonical_bson": "18000000136400000000000a5bc138938d44c64d31cc3700",
"degenerate_extjson": "{\"d\" : {\"$numberDecimal\" : \"1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000\"}}",
"canonical_extjson": "{\"d\" : {\"$numberDecimal\" : \"1.000000000000000000000000000000000E+999\"}}"
},
{
"description": "Clamped zeros with a large positive exponent",
"canonical_bson": "180000001364000000000000000000000000000000FE5F00",
"degenerate_extjson": "{\"d\" : {\"$numberDecimal\" : \"0E+2147483647\"}}",
"canonical_extjson": "{\"d\" : {\"$numberDecimal\" : \"0E+6111\"}}"
},
{
"description": "Clamped zeros with a large negative exponent",
"canonical_bson": "180000001364000000000000000000000000000000000000",
"degenerate_extjson": "{\"d\" : {\"$numberDecimal\" : \"0E-2147483647\"}}",
"canonical_extjson": "{\"d\" : {\"$numberDecimal\" : \"0E-6176\"}}"
},
{
"description": "Clamped negative zeros with a large positive exponent",
"canonical_bson": "180000001364000000000000000000000000000000FEDF00",
"degenerate_extjson": "{\"d\" : {\"$numberDecimal\" : \"-0E+2147483647\"}}",
"canonical_extjson": "{\"d\" : {\"$numberDecimal\" : \"-0E+6111\"}}"
},
{
"description": "Clamped negative zeros with a large negative exponent",
"canonical_bson": "180000001364000000000000000000000000000000008000",
"degenerate_extjson": "{\"d\" : {\"$numberDecimal\" : \"-0E-2147483647\"}}",
"canonical_extjson": "{\"d\" : {\"$numberDecimal\" : \"-0E-6176\"}}"
}
]
}
94 changes: 94 additions & 0 deletions src/libbson/tests/test-json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3555,6 +3555,97 @@ test_bson_as_json_with_opts_all_types (void)
bson_destroy (&scope);
}

static void
test_decimal128_overflowing_exponent (void)
{
{
bson_decimal128_t decimal128;
BSON_ASSERT (!bson_decimal128_from_string ("0E+2147483648", &decimal128));
}

{
bson_decimal128_t decimal128;
BSON_ASSERT (!bson_decimal128_from_string ("0E-2147483649", &decimal128));
}

{
bson_decimal128_t decimal128;
BSON_ASSERT (
!bson_decimal128_from_string ("-0E+2147483648", &decimal128));
}
{
bson_decimal128_t decimal128;
BSON_ASSERT (
!bson_decimal128_from_string ("-0E-2147483649", &decimal128));
}
{
bson_error_t error;
const char *degenerate_extjson =
"{\"d\" : {\"$numberDecimal\" : \"0E+2147483648\"}}";

BSON_ASSERT (!bson_new_from_json (
(const uint8_t *) degenerate_extjson, -1, &error));
ASSERT_ERROR_CONTAINS (
error,
BSON_ERROR_JSON,
BSON_JSON_ERROR_READ_INVALID_PARAM,
"Invalid input string \"0E+2147483648\", looking for DECIMAL128");
}

{
bson_error_t error;
const char *degenerate_extjson =
"{\"d\" : {\"$numberDecimal\" : \"0E-2147483649\"}}";

BSON_ASSERT (!bson_new_from_json (
(const uint8_t *) degenerate_extjson, -1, &error));
ASSERT_ERROR_CONTAINS (
error,
BSON_ERROR_JSON,
BSON_JSON_ERROR_READ_INVALID_PARAM,
"Invalid input string \"0E-2147483649\", looking for DECIMAL128");
}

{
bson_error_t error;
const char *degenerate_extjson =
"{\"d\" : {\"$numberDecimal\" : \"-0E+2147483648\"}}";

BSON_ASSERT (!bson_new_from_json (
(const uint8_t *) degenerate_extjson, -1, &error));
ASSERT_ERROR_CONTAINS (
error,
BSON_ERROR_JSON,
BSON_JSON_ERROR_READ_INVALID_PARAM,
"Invalid input string \"-0E+2147483648\", looking for DECIMAL128");
}

{
bson_error_t error;
const char *degenerate_extjson =
"{\"d\" : {\"$numberDecimal\" : \"-0E-2147483649\"}}";

BSON_ASSERT (!bson_new_from_json (
(const uint8_t *) degenerate_extjson, -1, &error));
ASSERT_ERROR_CONTAINS (
error,
BSON_ERROR_JSON,
BSON_JSON_ERROR_READ_INVALID_PARAM,
"Invalid input string \"-0E-2147483649\", looking for DECIMAL128");
}

{
bson_decimal128_t decimal128;
BSON_ASSERT (
!bson_decimal128_from_string ("0E+99999999999999999999", &decimal128));
}
{
bson_decimal128_t decimal128;
BSON_ASSERT (
!bson_decimal128_from_string ("0E-99999999999999999999", &decimal128));
}
}

void
test_json_install (TestSuite *suite)
{
Expand Down Expand Up @@ -3748,4 +3839,7 @@ test_json_install (TestSuite *suite)
TestSuite_Add (suite,
"/bson/as_json_with_opts/all_types",
test_bson_as_json_with_opts_all_types);
TestSuite_Add (suite,
"/bson/decimal128_overflowing_exponent",
test_decimal128_overflowing_exponent);
}