Sec Bug->Bug #71629 [Opn]: Out-of-bounds access in php_url_decode in context php_stream_url_wrap_rfc2397

Edit report at https://bugs.php.net/bug.php?id=71629&edit=1 ID: 71629 Updated by: stas@php.net Reported by: mt at debian dot org Summary: Out-of-bounds access in php_url_decode in context php_stream_url_wrap_rfc2397 Status: Open -Type: Security +Type: Bug Package: URL related Operating System: Linux/all PHP Version: 7.0.3 Block user comment: N Private report: Y New Comment: Don't see security issue here. Definition should be fixed though. Previous Comments: ------------------------------------------------------------------------ [2016-02-19 12:03:05] 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-19 11:39:18] 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 11:36:34] 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 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=71629&edit=1