php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #80966 Fishy code in ext/standard/browscap.c
Submitted: 2021-04-18 07:32 UTC Modified: 2021-04-20 10:31 UTC
From: lylgood at foxmail dot com Assigned:
Status: Open Package: *Extensibility Functions
PHP Version: master-Git-2021-04-18 (Git) OS: All
Private report: No CVE-ID: None
View Developer Edit
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: lylgood at foxmail dot com
New email:
PHP Version: OS:

 

 [2021-04-18 07:32 UTC] lylgood at foxmail dot com 
Description:
------------
File: ext/standard/browscap.c
Bug Function: php_browscap_parser_cb

In function php_browscap_parser_cb, pattern is re-assigned by pattern = zend_new_interned_string() at line 368.
Then if ZSTR_IS_INTERNED(pattern) is false, pattern will be freed via zend_string_release(pattern) at line 372.

But after that, pattern is still used at line 378 by zend_hash_update_ptr(bdata->htab, pattern, entry), which is a use after free bug.


Test script:
---------------
	if (persistent) {
368:		pattern = zend_new_interned_string(zend_string_copy(pattern));
		if (ZSTR_IS_INTERNED(pattern)) {
			Z_TYPE_FLAGS_P(arg1) = 0;
		} else {
372:			zend_string_release(pattern); //pattern could be freed !
		}
	}
        ...
378:    zend_hash_update_ptr(bdata->htab, pattern, entry);//freed pattern is used !

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-04-19 11:13 UTC] cmb@php.net
-Status: Open +Status: Feedback -Assigned To: +Assigned To: cmb
 [2021-04-19 11:13 UTC] cmb@php.net 
That code looks correct to me, since the pattern is copied, so
needs to be released if interning fails.  Could you come up with a
reproduce script showing the use-after-free?
 [2021-04-20 03:00 UTC] lylgood at foxmail dot com
-Status: Feedback +Status: Assigned
 [2021-04-20 03:00 UTC] lylgood at foxmail dot com 
This bug is reported by a code analyzer. Did you mean the pattern return via zend_new_interned_string() will hold ZSTR_IS_INTERNED(pattern) is true, and will not run into zend_string_release(pattern) ?
 [2021-04-20 10:31 UTC] cmb@php.net
-Summary: A potential use after free bug in ext/standard/browscap.c +Summary: Fishy code in ext/standard/browscap.c -Status: Assigned +Status: Open -Assigned To: cmb +Assigned To:
 [2021-04-20 10:31 UTC] cmb@php.net 
Before the if (persistent), `pattern` has a refcount >= 1. This is
increased by `zend_string_copy()`, so the refcount is > 1.  If
interning fails, the refcount of `pattern` is decreased by 1, so
it is still >= 1, i.e. `pattern` won't be freed.  Thus, I don't
see a potential use-after-free here.

However, that code looks fishy, and likely could be cleaned up.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Sun Dec 22 01:01:30 2024 UTC