Skip to content

Heap Buffer Overflow in zval_undefined_cv. #11028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Changochen opened this issue Apr 7, 2023 · 5 comments
Closed

Heap Buffer Overflow in zval_undefined_cv. #11028

Changochen opened this issue Apr 7, 2023 · 5 comments

Comments

@Changochen
Copy link

Description

The following code:

<?php
$c = (function () {
    [
        ...($r = (function () {
            try {
                yield $a => 0;
            } finally {
                return [];
            }
        })()),
    ];
})()[0];

with USE_TRACKED_ALLOC=1 USE_ZEND_ALLOC=0
Resulted in this output:

heap-buffer-overflow read

PHP Version

PHP 8.3.0-dev

Operating System

No response

@nielsdos
Copy link
Member

nielsdos commented Apr 7, 2023

Actually this seems to be a whole category of "yield"-related issues with finally's. This also doesn't work:

<?php
function test() {
    try {
        yield null => 0;
    } finally {
        return [];
    }
}

echo "hi\n";
var_dump([...test()]);
echo "hi2\n";

and if you replace null with (new stdClass), or false, or true, ... for example, it also doesn't work.

EDIT: progress....

zend_iterator_dtor(iter);
commenting this line "fixes" both my test case and OP's for some reason 🤔 (but ofc introduces leakage etc etc)

@nielsdos
Copy link
Member

nielsdos commented Apr 8, 2023

I attempted a fix here: nielsdos@6650263
This commit fixes both OP's issue and the issue I reported in my previous comment.

However, I found a variant of my previously reported issue which still crashes (may be different root cause?). It can be found here: nielsdos@6650263#diff-e61ec0f8972fbb546701b53555d59a0bfe720abdf3040f6b87c23a8a7f94db40
I'm not sure what's going on in that case tbh

@iluuu1994
Copy link
Member

Maybe @bwoebi or @arnaud-lb can help, they've touched this code most recently 🙂

@arnaud-lb
Copy link
Member

This script seems to result in a memory corruption since at least 7.4 according to https://3v4l.org/8SDGE

The problem seems to be that we discard an exception that was thrown by an other frame, but the frame will still try to handle it. Since EG(exception) is NULL, it will not take the right code path, and continue to execute after having freed live variables.

  • ZEND_ADD_ARRAY_UNPACK throws an Error("Keys must be of type int|string during array unpacking"). EG(exception) is set, and EG(current_execute_data)->opline is set to EG(exception_op)
  • The generator is closed, which resumes in the finally block
  • ZEND_DISCARD_EXCEPTION clears EG(exception), but the execute_data->opline of the parent frame remains EG(exception_op)
  • When resuming the parent frame, we are in an unexpected state: execute_data->opline is EG(exception_op), but EG(exception) is NULL
  • ZEND_HANDLE_EXCEPTION_SPEC_HANDLER cleanups live variables (including the array being built), and calls zend_leave_helper_SPEC
  • In zend_leave_helper_SPEC jumps to the next opline (ZEND_FETCH_DIM) because EG(exception) is NULL

@nielsdos your fix looks good. The case that still crashes is because EG(opline_before_exception) points to the generator op array, so in ZEND_HANDLE_EXCEPTION we call cleanup_unfinished_calls with an invalid op_num. We should probably save/restore EG(opline_before_exception) as well.

@nielsdos
Copy link
Member

Thanks. That makes sense. I missed the opline_before_exception part so that's why I didn't understand what was going on with my extra testcase.
I'll update the fix and PR it when I have time.

nielsdos added a commit to nielsdos/php-src that referenced this issue Apr 15, 2023
nielsdos added a commit to nielsdos/php-src that referenced this issue Apr 15, 2023
nielsdos added a commit to nielsdos/php-src that referenced this issue Apr 15, 2023
nielsdos added a commit that referenced this issue Apr 15, 2023
* PHP-8.1:
  Fix GH-11028: Heap Buffer Overflow in zval_undefined_cv.
nielsdos added a commit that referenced this issue Apr 15, 2023
* PHP-8.2:
  Fix GH-11028: Heap Buffer Overflow in zval_undefined_cv.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
@arnaud-lb @iluuu1994 @nielsdos @Changochen and others