From 48379fa237d350aa0a2d8e48bf2b18be7253a0ac Mon Sep 17 00:00:00 2001
From: Antony Dovgal <tony@daylessday.org>
Date: Tue, 11 Sep 2012 18:02:33 +0400
Subject: [PATCH] Issue #34: fix uninitialized read in runkit_method_copy()

set jmp_addr only for opcodes that actually have it and use it

new test was added
---
 runkit_functions.c                        | 53 +++++++++++++++--------
 tests/runkit_method_copy_uninit_read.phpt | 20 +++++++++
 2 files changed, 56 insertions(+), 17 deletions(-)
 create mode 100644 tests/runkit_method_copy_uninit_read.phpt

diff --git a/runkit_functions.c b/runkit_functions.c
index a9dd862..59cd435 100644
--- a/runkit_functions.c
+++ b/runkit_functions.c
@@ -130,7 +130,7 @@ void php_runkit_function_copy_ctor(zend_function *fe, const char *newname, int n
 #if PHP_MAJOR_VERSION > 5 || (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION >= 1)
 	zend_compiled_variable *dupvars;
 #endif
-	zend_op *opcode_copy;
+	zend_op *opcode_copy, *last_op;
 	int i;
 
 	if (newname) {
@@ -178,6 +178,7 @@ void php_runkit_function_copy_ctor(zend_function *fe, const char *newname, int n
 #endif
 
 		opcode_copy = safe_emalloc(sizeof(zend_op), fe->op_array.last, 0);
+		last_op = fe->op_array.opcodes + fe->op_array.last;
 		for(i = 0; i < fe->op_array.last; i++) {
 			opcode_copy[i] = fe->op_array.opcodes[i];
 #if (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION < 4) || (PHP_MAJOR_VERSION < 5)
@@ -188,17 +189,23 @@ void php_runkit_function_copy_ctor(zend_function *fe, const char *newname, int n
 #endif
 #ifdef ZEND_ENGINE_2
 			} else {
+				switch (opcode_copy[i].opcode) {
+# ifdef ZEND_GOTO
+					case ZEND_GOTO:
+# endif
+					case ZEND_JMP:
 #if (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION >= 4) || (PHP_MAJOR_VERSION > 5)
-				if (opcode_copy[i].op1.jmp_addr >= fe->op_array.opcodes &&
-				    opcode_copy[i].op1.jmp_addr <  fe->op_array.opcodes + fe->op_array.last) {
-					opcode_copy[i].op1.jmp_addr =  opcode_copy + (fe->op_array.opcodes[i].op1.jmp_addr - fe->op_array.opcodes);
-				}
+						if (opcode_copy[i].op1.jmp_addr >= fe->op_array.opcodes &&
+							opcode_copy[i].op1.jmp_addr < last_op) {
+							opcode_copy[i].op1.jmp_addr = opcode_copy + (fe->op_array.opcodes[i].op1.jmp_addr - fe->op_array.opcodes);
+						}
 #else
-				if (opcode_copy[i].op1.u.jmp_addr >= fe->op_array.opcodes &&
-				    opcode_copy[i].op1.u.jmp_addr <  fe->op_array.opcodes + fe->op_array.last) {
-					opcode_copy[i].op1.u.jmp_addr =  opcode_copy +     (fe->op_array.opcodes[i].op1.u.jmp_addr - fe->op_array.opcodes);
-				}
+						if (opcode_copy[i].op1.u.jmp_addr >= fe->op_array.opcodes &&
+							opcode_copy[i].op1.u.jmp_addr < last_op) {
+							opcode_copy[i].op1.u.jmp_addr = opcode_copy + (fe->op_array.opcodes[i].op1.u.jmp_addr - fe->op_array.opcodes);
+						}
 #endif
+				}
 #endif
 			}
 #if (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION < 4) || (PHP_MAJOR_VERSION < 5)
@@ -209,17 +216,29 @@ void php_runkit_function_copy_ctor(zend_function *fe, const char *newname, int n
 #endif
 #ifdef ZEND_ENGINE_2
 			} else {
+				switch (opcode_copy[i].opcode) {
+					case ZEND_JMPZ:
+					case ZEND_JMPNZ:
+					case ZEND_JMPZ_EX:
+					case ZEND_JMPNZ_EX:
+# ifdef ZEND_JMP_SET
+					case ZEND_JMP_SET:
+# endif
+# ifdef ZEND_JMP_SET_VAR
+					case ZEND_JMP_SET_VAR:
+# endif
 #if (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION >= 4) || (PHP_MAJOR_VERSION > 5)
-				if (opcode_copy[i].op2.jmp_addr >= fe->op_array.opcodes &&
-				    opcode_copy[i].op2.jmp_addr <  fe->op_array.opcodes + fe->op_array.last) {
-					opcode_copy[i].op2.jmp_addr =  opcode_copy + (fe->op_array.opcodes[i].op2.jmp_addr - fe->op_array.opcodes);
-				}
+						if (opcode_copy[i].op2.jmp_addr >= fe->op_array.opcodes &&
+							opcode_copy[i].op2.jmp_addr < last_op) {
+							opcode_copy[i].op2.jmp_addr =  opcode_copy + (fe->op_array.opcodes[i].op2.jmp_addr - fe->op_array.opcodes);
+						}
 #else
-				if (opcode_copy[i].op2.u.jmp_addr >= fe->op_array.opcodes &&
-				    opcode_copy[i].op2.u.jmp_addr <  fe->op_array.opcodes + fe->op_array.last) {
-					opcode_copy[i].op2.u.jmp_addr =  opcode_copy +     (fe->op_array.opcodes[i].op2.u.jmp_addr - fe->op_array.opcodes);
-				}
+						if (opcode_copy[i].op2.u.jmp_addr >= fe->op_array.opcodes &&
+							opcode_copy[i].op2.u.jmp_addr < last_op) {
+							opcode_copy[i].op2.u.jmp_addr =  opcode_copy + (fe->op_array.opcodes[i].op2.u.jmp_addr - fe->op_array.opcodes);
+						}
 #endif
+				}
 #endif
 			}
 		}
diff --git a/tests/runkit_method_copy_uninit_read.phpt b/tests/runkit_method_copy_uninit_read.phpt
new file mode 100644
index 0000000..d60b9ec
--- /dev/null
+++ b/tests/runkit_method_copy_uninit_read.phpt
@@ -0,0 +1,20 @@
+--TEST--
+runkit_method_copy() causes uninitialized read 
+--FILE--
+<?php
+
+runkit_method_copy('test', "new_method", "test", "old_method");
+
+class test
+{
+    function old_method() {
+            if(1)
+                $this->a[] = "b";
+    }
+}
+
+echo "==DONE==\n";
+
+?>
+--EXPECT--
+==DONE==