Skip to content

ext/bcmath: Prevent overflow of uint32_t/uint64_t #14297

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 3 commits into from
May 22, 2024
Merged

ext/bcmath: Prevent overflow of uint32_t/uint64_t #14297

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a1de07
ext/bcmath: Prevent overflow of uint32_t/uint64_t.
SakiTakamachi May 22, 2024
9d10968
Changed the definition method of BC_MUL_MAX_ADD_COUNT
SakiTakamachi May 22, 2024
73f5683
removed limits.h
SakiTakamachi May 22, 2024
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
31 changes: 25 additions & 6 deletions ext/bcmath/libbcmath/src/recmul.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,32 @@
#include <stddef.h>
#include <assert.h>
#include <stdbool.h>
#include <limits.h>
#include "private.h" /* For _bc_rm_leading_zeros() */
#include "zend_alloc.h"


#if SIZEOF_SIZE_T >= 8
# define BC_MUL_UINT_DIGITS 8
# define BC_MUL_UINT_OVERFLOW 100000000
# define BC_MUL_UINT_OVERFLOW (BC_UINT_T) 100000000
# define BC_MUL_MAX_ADD_COUNT (ULONG_MAX / (BC_MUL_UINT_OVERFLOW * BC_MUL_UINT_OVERFLOW))
Copy link
Member

Choose a reason for hiding this comment

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

Note that the size of a long is not necessarily the size of size_t.
I think I prefer a single definition:
#define BC_MUL_MAX_ADD_COUNT (~((BC_UINT_T) 0) / (BC_MUL_UINT_OVERFLOW * BC_MUL_UINT_OVERFLOW))

Copy link
Member Author

Choose a reason for hiding this comment

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

It's cool! thx

#else
# define BC_MUL_UINT_DIGITS 4
# define BC_MUL_UINT_OVERFLOW 10000
# define BC_MUL_UINT_OVERFLOW (BC_UINT_T) 10000
# define BC_MUL_MAX_ADD_COUNT (UINT_MAX / (BC_MUL_UINT_OVERFLOW * BC_MUL_UINT_OVERFLOW))
#endif


/* Multiply utility routines */

static inline void bc_digits_adjustment(BC_UINT_T *prod_uint, size_t prod_arr_size)
{
for (size_t i = 0; i < prod_arr_size - 1; i++) {
prod_uint[i + 1] += prod_uint[i] / BC_MUL_UINT_OVERFLOW;
prod_uint[i] %= BC_MUL_UINT_OVERFLOW;
}
}

/*
* Converts BCD to uint, going backwards from pointer n by the number of
* characters specified by len.
Expand Down Expand Up @@ -141,7 +152,18 @@ static void bc_standard_mul(bc_num n1, size_t n1len, bc_num n2, size_t n2len, bc
bc_convert_to_uint(n2_uint, n2end, n2len);

/* Multiplication and addition */
size_t count = 0;
for (i = 0; i < n1_arr_size; i++) {
/*
* This calculation adds the result multiple times to the array entries.
* When multiplying large numbers of digits, there is a possibility of
* overflow, so digit adjustment is performed beforehand.
*/
if (UNEXPECTED(count >= BC_MUL_MAX_ADD_COUNT)) {
bc_digits_adjustment(prod_uint, prod_arr_size);
count = 0;
}
count++;
for (size_t j = 0; j < n2_arr_size; j++) {
prod_uint[i + j] += n1_uint[i] * n2_uint[j];
}
Expand All @@ -151,10 +173,7 @@ static void bc_standard_mul(bc_num n1, size_t n1len, bc_num n2, size_t n2len, bc
* Move a value exceeding 4/8 digits by carrying to the next digit.
* However, the last digit does nothing.
*/
for (i = 0; i < prod_arr_size - 1; i++) {
prod_uint[i + 1] += prod_uint[i] / BC_MUL_UINT_OVERFLOW;
prod_uint[i] %= BC_MUL_UINT_OVERFLOW;
}
bc_digits_adjustment(prod_uint, prod_arr_size);

/* Convert to bc_num */
*prod = bc_new_num_nonzeroed(prodlen, 0);
Expand Down
35 changes: 35 additions & 0 deletions ext/bcmath/tests/bcmul_check_overflow.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
--TEST--
bcmul() checking overflow
--EXTENSIONS--
bcmath
--INI--
bcmath.scale=0
--FILE--
<?php
for ($i = 1; $i < 15; $i++) {
$repeat = 2 ** $i;
$num1 = str_repeat('99999999', $repeat);
/*
* 9999 * 9999 = 99980001
* 99999999 * 99999999 = 9999999800000001
*/
$expected = str_repeat('9', $repeat * 8 - 1) . '8' . str_repeat('0', $repeat * 8 - 1) . '1';
$actual = bcmul($num1, $num1);
echo $repeat . ': ' . ($actual === $expected ? 'OK' : 'NG') . PHP_EOL;
}
?>
--EXPECT--
2: OK
4: OK
8: OK
16: OK
32: OK
64: OK
128: OK
256: OK
512: OK
1024: OK
2048: OK
4096: OK
8192: OK
16384: OK
Loading