Browse files

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
  • Loading branch information...
1 parent 48426c7 commit 48379fa237d350aa0a2d8e48bf2b18be7253a0ac @tony2001 tony2001 committed with Sep 11, 2012
Showing with 56 additions and 17 deletions.
  1. +36 −17 runkit_functions.c
  2. +20 −0 tests/runkit_method_copy_uninit_read.phpt
View
53 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
}
}
View
20 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==

0 comments on commit 48379fa

Please sign in to comment.