Skip to content

fix: dangling opline in ZEND_INIT_ARRAY #18578

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

Merged
merged 1 commit into from
May 19, 2025

Conversation

morrisonlevi
Copy link
Contributor

@morrisonlevi morrisonlevi commented May 16, 2025

This causes problems if an allocation profiler decides to walk the stack, or if the engine itself OOMs on this opcode, and it tries to print file and line information.

This is similar to #12648 and #12758. Targeting PHP 8.3.

I don't have a reproducer for this one yet. You can see from prior issues that you have to manipulate the frames and such, so it's a bit tricky. I have two customer crash reports with ZEND_INIT_ARRAY_SPEC_TMP_UNUSED_HANDLER, so I know it's reachable, I just haven't figured out exactly how to do reproduce it yet.

This causes problems if an allocation profiler decides to walk the
stack, or if the engine itself OOMs on this opcode, and it tries to
print file and line information.
Copy link
Member

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

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

SAVE_OPLINE is only needed for zend_hash_real_init_mixed.

@morrisonlevi
Copy link
Contributor Author

morrisonlevi commented May 18, 2025

SAVE_OPLINE is only needed for zend_hash_real_init_mixed.

No, it's any allocation. The crash reports have zend_new_array in them (I do not know which branch, it's in there twice and I don't have line numbers). I'm guessing here, but since one of the crashes is in ZEND_INIT_ARRAY_SPEC_TMP_UNUSED_HANDLER, I would guess that op1 is TMP and therefore OP1_TYPE != IS_UNUSED on line 6441 is true, and it fails on ZVAL_ARR(array, zend_new_array(size)) on line 6444.

6435  ZEND_VM_HANDLER(71, ZEND_INIT_ARRAY, CONST|TMP|VAR|CV|UNUSED, CONST|TMPVAR|UNUSED|NEXT|CV, ARRAY_INIT|REF)
6436  {
6437  	zval *array;
6438  	uint32_t size;
6439  	USE_OPLINE
6440  
      	SAVE_OPLINE();
6441  	array = EX_VAR(opline->result.var);
6442  	if (OP1_TYPE != IS_UNUSED) {
6443  		size = opline->extended_value >> ZEND_ARRAY_SIZE_SHIFT;
6444  		ZVAL_ARR(array, zend_new_array(size));
6445  		/* Explicitly initialize array as not-packed if flag is set */
6446  		if (opline->extended_value & ZEND_ARRAY_NOT_PACKED) {
6447  			zend_hash_real_init_mixed(Z_ARRVAL_P(array));
6448  		}
6449  		ZEND_VM_DISPATCH_TO_HANDLER(ZEND_ADD_ARRAY_ELEMENT);
6450  	} else {
6451  		ZVAL_ARR(array, zend_new_array(0));
6452  		ZEND_VM_NEXT_OPCODE();
6453  	}
6454  }

Copy link
Member

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

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

You are right, I must have brainfarted.

@morrisonlevi morrisonlevi merged commit 35455b1 into php:PHP-8.3 May 19, 2025
9 checks passed
@morrisonlevi morrisonlevi deleted the ZEND_INIT_ARRAY branch May 19, 2025 15:45
nielsdos added a commit that referenced this pull request May 19, 2025
* PHP-8.4:
  Fix OSS-Fuzz #418106144
  Fix OSS-Fuzz #417078295
  fix: dangling opline in ZEND_INIT_ARRAY (#18578)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants