Fixed bug #80545

This converts the remaining "non well-formed" warnings in bcmath to ValueErrors, in line with the other warning promotions that have been performed in this extension. Closes GH-80545.
php · Jan 12, 2021 · 94a151a · 94a151a
1 parent 95a13ca
commit 94a151a
Show file tree

Hide file tree

Showing 4 changed files with 248 additions and 84 deletions.
diff --git a/NEWS b/NEWS
@@ -7,6 +7,10 @@ PHP                                                                        NEWS
   . Fixed bug #80384 (filter buffers entire read until file closed). (Adam
     Seitz, cmb)
 
+- BCMath:
+  . Fixed bug #80545 (bcadd('a', 'a') doesn't throw an exception).
+    (Jens de Nies)
+
 - Date:
   . Fixed bug #80376 (last day of the month causes runway cpu usage). (Derick)
 

diff --git a/ext/bcmath/bcmath.c b/ext/bcmath/bcmath.c
@@ -129,20 +129,23 @@ PHP_MINFO_FUNCTION(bcmath)
 
 /* {{{ php_str2num
    Convert to bc_num detecting scale */
-static void php_str2num(bc_num *num, char *str)
+static zend_result php_str2num(bc_num *num, char *str)
 {
 	char *p;
 
 	if (!(p = strchr(str, '.'))) {
 		if (!bc_str2num(num, str, 0)) {
-			php_error_docref(NULL, E_WARNING, "bcmath function argument is not well-formed");
+			return FAILURE;
 		}
-		return;
+
+		return SUCCESS;
 	}
 
 	if (!bc_str2num(num, str, strlen(p+1))) {
-		php_error_docref(NULL, E_WARNING, "bcmath function argument is not well-formed");
+		return FAILURE;
 	}
+
+	return SUCCESS;
 }
 /* }}} */
 
@@ -174,15 +177,26 @@ PHP_FUNCTION(bcadd)
 	bc_init_num(&first);
 	bc_init_num(&second);
 	bc_init_num(&result);
-	php_str2num(&first, ZSTR_VAL(left));
-	php_str2num(&second, ZSTR_VAL(right));
+
+	if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
+		zend_argument_value_error(1, "is not well-formed");
+		goto cleanup;
+	}
+
+	if (php_str2num(&second, ZSTR_VAL(right)) == FAILURE) {
+		zend_argument_value_error(2, "is not well-formed");
+		goto cleanup;
+	}
+
 	bc_add (first, second, &result, scale);
 
 	RETVAL_STR(bc_num2str_ex(result, scale));
-	bc_free_num(&first);
-	bc_free_num(&second);
-	bc_free_num(&result);
-	return;
+
+	cleanup: {
+		bc_free_num(&first);
+		bc_free_num(&second);
+		bc_free_num(&result);
+	};
 }
 /* }}} */
 
@@ -214,15 +228,26 @@ PHP_FUNCTION(bcsub)
 	bc_init_num(&first);
 	bc_init_num(&second);
 	bc_init_num(&result);
-	php_str2num(&first, ZSTR_VAL(left));
-	php_str2num(&second, ZSTR_VAL(right));
+
+	if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
+		zend_argument_value_error(1, "is not well-formed");
+		goto cleanup;
+	}
+
+	if (php_str2num(&second, ZSTR_VAL(right)) == FAILURE) {
+		zend_argument_value_error(2, "is not well-formed");
+		goto cleanup;
+	}
+
 	bc_sub (first, second, &result, scale);
 
 	RETVAL_STR(bc_num2str_ex(result, scale));
-	bc_free_num(&first);
-	bc_free_num(&second);
-	bc_free_num(&result);
-	return;
+
+	cleanup: {
+		bc_free_num(&first);
+		bc_free_num(&second);
+		bc_free_num(&result);
+	};
 }
 /* }}} */
 
@@ -254,15 +279,26 @@ PHP_FUNCTION(bcmul)
 	bc_init_num(&first);
 	bc_init_num(&second);
 	bc_init_num(&result);
-	php_str2num(&first, ZSTR_VAL(left));
-	php_str2num(&second, ZSTR_VAL(right));
+
+	if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
+		zend_argument_value_error(1, "is not well-formed");
+		goto cleanup;
+	}
+
+	if (php_str2num(&second, ZSTR_VAL(right)) == FAILURE) {
+		zend_argument_value_error(2, "is not well-formed");
+		goto cleanup;
+	}
+
 	bc_multiply (first, second, &result, scale);
 
 	RETVAL_STR(bc_num2str_ex(result, scale));
-	bc_free_num(&first);
-	bc_free_num(&second);
-	bc_free_num(&result);
-	return;
+
+	cleanup: {
+		bc_free_num(&first);
+		bc_free_num(&second);
+		bc_free_num(&result);
+	};
 }
 /* }}} */
 
@@ -294,8 +330,16 @@ PHP_FUNCTION(bcdiv)
 	bc_init_num(&first);
 	bc_init_num(&second);
 	bc_init_num(&result);
-	php_str2num(&first, ZSTR_VAL(left));
-	php_str2num(&second, ZSTR_VAL(right));
+
+	if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
+		zend_argument_value_error(1, "is not well-formed");
+		goto cleanup;
+	}
+
+	if (php_str2num(&second, ZSTR_VAL(right)) == FAILURE) {
+		zend_argument_value_error(2, "is not well-formed");
+		goto cleanup;
+	}
 
 	switch (bc_divide(first, second, &result, scale)) {
 		case 0: /* OK */
@@ -306,10 +350,11 @@ PHP_FUNCTION(bcdiv)
 			break;
 	}
 
-	bc_free_num(&first);
-	bc_free_num(&second);
-	bc_free_num(&result);
-	return;
+	cleanup: {
+		bc_free_num(&first);
+		bc_free_num(&second);
+		bc_free_num(&result);
+	};
 }
 /* }}} */
 
@@ -341,8 +386,16 @@ PHP_FUNCTION(bcmod)
 	bc_init_num(&first);
 	bc_init_num(&second);
 	bc_init_num(&result);
-	php_str2num(&first, ZSTR_VAL(left));
-	php_str2num(&second, ZSTR_VAL(right));
+
+	if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
+		zend_argument_value_error(1, "is not well-formed");
+		goto cleanup;
+	}
+
+	if (php_str2num(&second, ZSTR_VAL(right)) == FAILURE) {
+		zend_argument_value_error(2, "is not well-formed");
+		goto cleanup;
+	}
 
 	switch (bc_modulo(first, second, &result, scale)) {
 		case 0:
@@ -353,9 +406,11 @@ PHP_FUNCTION(bcmod)
 			break;
 	}
 
-	bc_free_num(&first);
-	bc_free_num(&second);
-	bc_free_num(&result);
+	cleanup: {
+		bc_free_num(&first);
+		bc_free_num(&second);
+		bc_free_num(&result);
+	};
 }
 /* }}} */
 
@@ -389,18 +444,32 @@ PHP_FUNCTION(bcpowmod)
 	bc_init_num(&second);
 	bc_init_num(&mod);
 	bc_init_num(&result);
-	php_str2num(&first, ZSTR_VAL(left));
-	php_str2num(&second, ZSTR_VAL(right));
-	php_str2num(&mod, ZSTR_VAL(modulus));
+
+	if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
+		zend_argument_value_error(1, "is not well-formed");
+		goto cleanup;
+	}
+
+	if (php_str2num(&second, ZSTR_VAL(right)) == FAILURE) {
+		zend_argument_value_error(2, "is not well-formed");
+		goto cleanup;
+	}
+
+	if (php_str2num(&mod, ZSTR_VAL(modulus)) == FAILURE) {
+		zend_argument_value_error(3, "is not well-formed");
+		goto cleanup;
+	}
 
 	if (bc_raisemod(first, second, mod, &result, scale) == SUCCESS) {
 		RETVAL_STR(bc_num2str_ex(result, scale));
 	}
 
-	bc_free_num(&first);
-	bc_free_num(&second);
-	bc_free_num(&mod);
-	bc_free_num(&result);
+	cleanup: {
+		bc_free_num(&first);
+		bc_free_num(&second);
+		bc_free_num(&mod);
+		bc_free_num(&result);
+	};
 }
 /* }}} */
 
@@ -432,14 +501,26 @@ PHP_FUNCTION(bcpow)
 	bc_init_num(&first);
 	bc_init_num(&second);
 	bc_init_num(&result);
-	php_str2num(&first, ZSTR_VAL(left));
-	php_str2num(&second, ZSTR_VAL(right));
+
+	if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
+		zend_argument_value_error(1, "is not well-formed");
+		goto cleanup;
+	}
+
+	if (php_str2num(&second, ZSTR_VAL(right)) == FAILURE) {
+		zend_argument_value_error(2, "is not well-formed");
+		goto cleanup;
+	}
+
 	bc_raise (first, second, &result, scale);
 
 	RETVAL_STR(bc_num2str_ex(result, scale));
-	bc_free_num(&first);
-	bc_free_num(&second);
-	bc_free_num(&result);
+
+	cleanup: {
+		bc_free_num(&first);
+		bc_free_num(&second);
+		bc_free_num(&result);
+	};
 }
 /* }}} */
 
@@ -468,16 +549,21 @@ PHP_FUNCTION(bcsqrt)
 	}
 
 	bc_init_num(&result);
-	php_str2num(&result, ZSTR_VAL(left));
+
+	if (php_str2num(&result, ZSTR_VAL(left)) == FAILURE) {
+		zend_argument_value_error(1, "is not well-formed");
+		goto cleanup;
+	}
 
 	if (bc_sqrt (&result, scale) != 0) {
 		RETVAL_STR(bc_num2str_ex(result, scale));
 	} else {
 		zend_argument_value_error(1, "must be greater than or equal to 0");
 	}
 
-	bc_free_num(&result);
-	return;
+	cleanup: {
+		bc_free_num(&result);
+	};
 }
 /* }}} */
 
@@ -510,16 +596,21 @@ PHP_FUNCTION(bccomp)
 	bc_init_num(&second);
 
 	if (!bc_str2num(&first, ZSTR_VAL(left), scale)) {
-		php_error_docref(NULL, E_WARNING, "bcmath function argument is not well-formed");
+		zend_argument_value_error(1, "is not well-formed");
+		goto cleanup;
 	}
+
 	if (!bc_str2num(&second, ZSTR_VAL(right), scale)) {
-		php_error_docref(NULL, E_WARNING, "bcmath function argument is not well-formed");
+		zend_argument_value_error(2, "is not well-formed");
+		goto cleanup;
 	}
+
 	RETVAL_LONG(bc_compare(first, second));
 
-	bc_free_num(&first);
-	bc_free_num(&second);
-	return;
+	cleanup: {
+		bc_free_num(&first);
+		bc_free_num(&second);
+	};
 }
 /* }}} */
 

diff --git a/ext/bcmath/tests/bug80545.phpt b/ext/bcmath/tests/bug80545.phpt
@@ -0,0 +1,25 @@
+--TEST--
+Bug #80545 (bcadd('a', 'a') and bcadd('1', 'a') doesn't throw an exception)
+--SKIPIF--
+<?php
+if (!extension_loaded('bcmath')) die('skip bcmath extension not available');
+?>
+--FILE--
+<?php
+
+try {
+    bcadd('a', 'a');
+} catch (\ValueError $e) {
+    echo $e->getMessage() . PHP_EOL;
+}
+
+try {
+    bcadd('1', 'a');
+} catch (\ValueError $e) {
+    echo $e->getMessage();
+}
+
+?>
+--EXPECT--
+bcadd(): Argument #1 ($num1) is not well-formed
+bcadd(): Argument #2 ($num2) is not well-formed