Skip to content

Commit

Permalink
Fix leak on div by zero compound assignment with coercion
Browse files Browse the repository at this point in the history
The result == op1 check did not work properly here, because op1
was &op1_copy at this point. Move the division by zero reporting
out of the _base function, so it can check the original op1.
  • Loading branch information
nikic committed Jul 1, 2021
1 parent b976bc4 commit 540fed1
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 15 deletions.
14 changes: 14 additions & 0 deletions Zend/tests/div_by_zero_compound_with_conversion.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Division by zero in compound operator with type coercion
--FILE--
<?php
$x = 42;
try {
$$x /= 0;
} catch (DivisionByZeroError $e) {
echo $e->getMessage(), "\n";
}
?>
--EXPECTF--
Warning: Undefined variable $42 in %s on line %d
Division by zero
38 changes: 23 additions & 15 deletions Zend/zend_operators.c
Original file line number Diff line number Diff line change
Expand Up @@ -1253,15 +1253,16 @@ ZEND_API zend_result ZEND_FASTCALL pow_function(zval *result, zval *op1, zval *o
}
/* }}} */

/* Returns SUCCESS/FAILURE/TYPES_NOT_HANDLED */
/* Returns SUCCESS/TYPES_NOT_HANDLED/DIV_BY_ZERO */
#define TYPES_NOT_HANDLED 1
#define DIV_BY_ZERO 2
static int ZEND_FASTCALL div_function_base(zval *result, zval *op1, zval *op2) /* {{{ */
{
zend_uchar type_pair = TYPE_PAIR(Z_TYPE_P(op1), Z_TYPE_P(op2));

if (EXPECTED(type_pair == TYPE_PAIR(IS_LONG, IS_LONG))) {
if (Z_LVAL_P(op2) == 0) {
goto division_by_0;
return DIV_BY_ZERO;
} else if (Z_LVAL_P(op2) == -1 && Z_LVAL_P(op1) == ZEND_LONG_MIN) {
/* Prevent overflow error/crash */
ZVAL_DOUBLE(result, (double) ZEND_LONG_MIN / -1);
Expand All @@ -1275,31 +1276,25 @@ static int ZEND_FASTCALL div_function_base(zval *result, zval *op1, zval *op2) /
return SUCCESS;
} else if (EXPECTED(type_pair == TYPE_PAIR(IS_DOUBLE, IS_DOUBLE))) {
if (Z_DVAL_P(op2) == 0) {
goto division_by_0;
return DIV_BY_ZERO;
}
ZVAL_DOUBLE(result, Z_DVAL_P(op1) / Z_DVAL_P(op2));
return SUCCESS;
} else if (EXPECTED(type_pair == TYPE_PAIR(IS_DOUBLE, IS_LONG))) {
if (Z_LVAL_P(op2) == 0) {
goto division_by_0;
return DIV_BY_ZERO;
}
ZVAL_DOUBLE(result, Z_DVAL_P(op1) / (double)Z_LVAL_P(op2));
return SUCCESS;
} else if (EXPECTED(type_pair == TYPE_PAIR(IS_LONG, IS_DOUBLE))) {
if (Z_DVAL_P(op2) == 0) {
goto division_by_0;
return DIV_BY_ZERO;
}
ZVAL_DOUBLE(result, (double)Z_LVAL_P(op1) / Z_DVAL_P(op2));
return SUCCESS;
} else {
return TYPES_NOT_HANDLED;
}
division_by_0:
if (result != op1) {
ZVAL_UNDEF(result);
}
zend_throw_error(zend_ce_division_by_zero_error, "Division by zero");
return FAILURE;
}
/* }}} */

Expand All @@ -1309,8 +1304,12 @@ ZEND_API zend_result ZEND_FASTCALL div_function(zval *result, zval *op1, zval *o
ZVAL_DEREF(op2);

int retval = div_function_base(result, op1, op2);
if (retval != TYPES_NOT_HANDLED) {
return retval;
if (EXPECTED(retval == SUCCESS)) {
return SUCCESS;
}

if (UNEXPECTED(retval == DIV_BY_ZERO)) {
goto div_by_zero;
}

ZEND_TRY_BINARY_OBJECT_OPERATION(ZEND_DIV);
Expand All @@ -1330,8 +1329,17 @@ ZEND_API zend_result ZEND_FASTCALL div_function(zval *result, zval *op1, zval *o
}

retval = div_function_base(result, &op1_copy, &op2_copy);
ZEND_ASSERT(retval != TYPES_NOT_HANDLED && "Types should be handled");
return retval;
if (retval == SUCCESS) {
return SUCCESS;
}

div_by_zero:
ZEND_ASSERT(retval == DIV_BY_ZERO && "TYPES_NOT_HANDLED should not occur here");
if (result != op1) {
ZVAL_UNDEF(result);
}
zend_throw_error(zend_ce_division_by_zero_error, "Division by zero");
return FAILURE;
}
/* }}} */

Expand Down

1 comment on commit 540fed1

@gtt1995
Copy link

@gtt1995 gtt1995 commented on 540fed1 Jul 1, 2021

Choose a reason for hiding this comment

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

Thanks a lot .

Please sign in to comment.