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

go to bug id or search bugs for

Bug #71629 Out-of-bounds access in php_url_decode in context php_stream_url_wrap_rfc2397
Submitted: 2016-02-19 11:36 UTC Modified: 2016-02-22 01:02 UTC
From: mt at debian dot org Assigned:
Status: Closed Package: URL related
PHP Version: 7.0.3 OS: Linux/all
Private report: No CVE-ID: None
View Developer Edit
 [2016-02-19 11:36 UTC] mt at debian dot org 
Description:
------------
goto-cc (part of the cbmc package) reported that php_url_decode is defined with
size_t parameter and return types in ext/standard/url.c, but main/streams/memory.c for some reason runs its own declaration, using int.

For all systems with sizeof(int)!=sizeof(size_t), the sizeof(size_t)-sizeof(int) bytes will be taken from somewhere on the stack or an uninitialised register. Thus decoding may have some arbitrary behaviour, where a crash is likely the best possible case - it's an out-of-bounds memory access.

Best,
Michael

Patches

memory.patch (last revision 2016-02-19 12:03 UTC by ondrej@php.net)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-02-19 11:39 UTC] mt at debian dot org 
It seems I cannot add a patch to a security-tagged bug report (authentication error), hence posting it here:

diff -urN a/main/streams/memory.c b/main/streams/memory.c
--- a/main/streams/memory.c	2016-02-19 02:17:42.000000000 +0000
+++ b/main/streams/memory.c	2016-02-19 02:18:41.000000000 +0000
@@ -21,7 +21,7 @@
 #include "php.h"
 #include "ext/standard/base64.h"

-PHPAPI int php_url_decode(char *str, int len);
+PHPAPI size_t php_url_decode(char *str, size_t len);

 /* Memory streams use a dynamic memory buffer to emulate a stream.
  * You can use php_stream_memory_open to create a readonly stream
@@ -729,7 +729,7 @@
 		ilen = (int)ZSTR_LEN(base64_comma);
 	} else {
 		comma = estrndup(comma, dlen);
-		dlen = php_url_decode(comma, (int)dlen);
+		dlen = php_url_decode(comma, dlen);
 		ilen = (int)dlen;
 	}
 [2016-02-19 12:03 UTC] ondrej@php.net 
The following patch has been added/updated:

Patch Name: memory.patch
Revision:   1455883384
URL:        https://bugs.php.net/patch-display.php?bug=71629&patch=memory.patch&revision=1455883384
 [2016-02-22 01:02 UTC] stas@php.net
-Type: Security +Type: Bug
 [2016-02-22 01:02 UTC] stas@php.net 
Don't see security issue here. Definition should be fixed though.
 [2016-02-22 01:12 UTC] stas@php.net 
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=d25e67eee653c8ce8ce1a4459181e3b802eb915c
Log: Fix bug #71629: sync php_url_decode definition
 [2016-02-22 01:12 UTC] stas@php.net
-Status: Open +Status: Closed
 [2016-07-20 11:33 UTC] davey@php.net 
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=d25e67eee653c8ce8ce1a4459181e3b802eb915c
Log: Fix bug #71629: sync php_url_decode definition
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Sat Dec 21 16:01:28 2024 UTC