Fixed bug #44184 (Double free of loop-variable on exception)

5 files changed, 41 insertions, 11 deletions

diff --git a/NEWS b/NEWS
index b720bf12f7..b481394699 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,7 @@ PHP                                                                        NEWS
   which to group by data is specified. (Ilia)
 - Upgraded PCRE to version 7.6 (Nuno)
 
+- Fixed bug #44184 (Double free of loop-variable on exception). (Dmitry)
 - Fixed bug #44171 (Invalid FETCH_COLUMN index does not raise an error). (Ilia)
 - Fixed Bug #44159 (Crash: $pdo->setAttribute(PDO::STATEMENT_ATTR_CLASS, NULL)).
   (Felipe)
diff --git a/Zend/tests/bug44184.phpt b/Zend/tests/bug44184.phpt
new file mode 100644
index 0000000000..7f277acc74
--- /dev/null
+++ b/Zend/tests/bug44184.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Bug #44184 (Double free of loop-variable on exception)
+--FILE--
+<?php
+function foo() {
+	$x = array(1,2,3);
+	foreach ($x as $a) {
+		while (1) {
+			throw new Exception();
+		}
+	    return;
+	}
+}
+try {
+	foo();
+} catch (Exception $ex) {
+	echo "ok\n";
+}
+?>
+--EXPECT--
+ok
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index e8832af142..1bd3a4079a 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -685,8 +685,14 @@ static inline void do_begin_loop(TSRMLS_D)
 }
 
 
-static inline void do_end_loop(int cont_addr TSRMLS_DC)
+static inline void do_end_loop(int cont_addr, int has_loop_var TSRMLS_DC)
 {
+	if (!has_loop_var) {
+		/* The start fileld is used to free temporary variables in case of exceptions.
+		 * We won't try to free something of we don't have loop variable.
+		 */
+		CG(active_op_array)->brk_cont_array[CG(active_op_array)->current_brk_cont].start = -1;
+	}
 	CG(active_op_array)->brk_cont_array[CG(active_op_array)->current_brk_cont].cont = cont_addr;
 	CG(active_op_array)->brk_cont_array[CG(active_op_array)->current_brk_cont].brk = get_next_op_number(CG(active_op_array));
 	CG(active_op_array)->current_brk_cont = CG(active_op_array)->brk_cont_array[CG(active_op_array)->current_brk_cont].parent;
@@ -721,7 +727,7 @@ void zend_do_while_end(znode *while_token, znode *close_bracket_token TSRMLS_DC)
 	/* update while's conditional jmp */
 	CG(active_op_array)->opcodes[close_bracket_token->u.opline_num].op2.u.opline_num = get_next_op_number(CG(active_op_array));
 
-	do_end_loop(while_token->u.opline_num TSRMLS_CC);
+	do_end_loop(while_token->u.opline_num, 0 TSRMLS_CC);
 
 	DEC_BPC(CG(active_op_array));
 }
@@ -765,7 +771,7 @@ void zend_do_for_end(znode *second_semicolon_token TSRMLS_DC)
 	SET_UNUSED(opline->op1);
 	SET_UNUSED(opline->op2);
 
-	do_end_loop(second_semicolon_token->u.opline_num+1 TSRMLS_CC);
+	do_end_loop(second_semicolon_token->u.opline_num+1, 0 TSRMLS_CC);
 
 	DEC_BPC(CG(active_op_array));
 }
@@ -2646,7 +2652,7 @@ void zend_do_do_while_end(znode *do_token, znode *expr_open_bracket, znode *expr
 	opline->op2.u.opline_num = do_token->u.opline_num;
 	SET_UNUSED(opline->op2);
 
-	do_end_loop(expr_open_bracket->u.opline_num TSRMLS_CC);
+	do_end_loop(expr_open_bracket->u.opline_num, 0 TSRMLS_CC);
 
 	DEC_BPC(CG(active_op_array));
 }
@@ -3891,7 +3897,7 @@ void zend_do_foreach_end(znode *foreach_token, znode *as_token TSRMLS_DC)
 	CG(active_op_array)->opcodes[foreach_token->u.opline_num].op2.u.opline_num = get_next_op_number(CG(active_op_array)); /* FE_RESET */
 	CG(active_op_array)->opcodes[as_token->u.opline_num].op2.u.opline_num = get_next_op_number(CG(active_op_array)); /* FE_FETCH */
 
-	do_end_loop(as_token->u.opline_num TSRMLS_CC);
+	do_end_loop(as_token->u.opline_num, 1 TSRMLS_CC);
 
 	zend_stack_top(&CG(foreach_copy_stack), (void **) &container_ptr);
 	generate_free_foreach_copy(container_ptr TSRMLS_CC);
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
index 53b675e6d4..e9e5b3a7c6 100644
--- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -3826,11 +3826,12 @@ ZEND_VM_HANDLER(149, ZEND_HANDLE_EXCEPTION, ANY, ANY)
 	}
 
 	for (i=0; i<EX(op_array)->last_brk_cont; i++) {
-		if (EX(op_array)->brk_cont_array[i].start > op_num) {
+		if (EX(op_array)->brk_cont_array[i].start < 0) {
+			continue;
+		} else if (EX(op_array)->brk_cont_array[i].start > op_num) {
 			/* further blocks will not be relevant... */
 			break;
-		}
-		if (op_num < EX(op_array)->brk_cont_array[i].brk) {
+		} else if (op_num < EX(op_array)->brk_cont_array[i].brk) {
 			if (!catched ||
 			    catch_op_num >= EX(op_array)->brk_cont_array[i].brk) {
 				zend_op *brk_opline = &EX(op_array)->opcodes[EX(op_array)->brk_cont_array[i].brk];
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
index 6d7402f78b..0da39e33a5 100644
--- a/Zend/zend_vm_execute.h
+++ b/Zend/zend_vm_execute.h
@@ -566,11 +566,12 @@ static int ZEND_HANDLE_EXCEPTION_SPEC_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
 	}
 
 	for (i=0; i<EX(op_array)->last_brk_cont; i++) {
-		if (EX(op_array)->brk_cont_array[i].start > op_num) {
+		if (EX(op_array)->brk_cont_array[i].start < 0) {
+			continue;
+		} else if (EX(op_array)->brk_cont_array[i].start > op_num) {
 			/* further blocks will not be relevant... */
 			break;
-		}
-		if (op_num < EX(op_array)->brk_cont_array[i].brk) {
+		} else if (op_num < EX(op_array)->brk_cont_array[i].brk) {
 			if (!catched ||
 			    catch_op_num >= EX(op_array)->brk_cont_array[i].brk) {
 				zend_op *brk_opline = &EX(op_array)->opcodes[EX(op_array)->brk_cont_array[i].brk];