php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #73173 huge memleak when wddx_unserialize
Submitted: 2016-09-26 08:13 UTC Modified: 2016-09-26 10:17 UTC
Votes:1
Avg. Score:3.0 ± 0.0
Reproduced:0 of 0 (0.0%)
From: tloi at fortinet dot com Assigned:
Status: Closed Package: WDDX related
PHP Version: master-Git-2016-09-26 (Git) OS: All
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: tloi at fortinet dot com
New email:
PHP Version: OS:

 

 [2016-09-26 08:13 UTC] tloi at fortinet dot com
Description:
------------
If the xml document has <var name="xxx"> tag without </var>, the varname variable will never freed and causing memory leak equal to the strlen(varname).

<snippet func php_wddx_push_element>
	} else if (!strcmp((char *)name, EL_VAR)) {
		int i;

		if (atts) for (i = 0; atts[i]; i++) {
			if (!strcmp((char *)atts[i], EL_NAME) && atts[i+1] && atts[i+1][0]) {
				if (stack->varname) efree(stack->varname);
				stack->varname = estrdup((char *)atts[i+1]); //strdup the varname
				break;
			}
		}
	} else if (!strcmp((char *)name, EL_RECORDSET)) {
</snippet>

Patch is simple, free the varname when destroy the stack:

--- a/ext/wddx/wddx.c
+++ b/ext/wddx/wddx.c
@@ -241,6 +241,9 @@ static int wddx_stack_destroy(wddx_stack *stack)
                }
                efree(stack->elements);
        }
+       if (stack->varname) {
+               efree(stack->varname);
+       }
        return SUCCESS;
 }
 /* }}} */

This bug may lead to DoS on the server as the leak is significant for each request, the poc is leaking 8MB of memory without any special configuration.
compiled with only --enable-debug --enable-wddx

Test script:
---------------
<?php
$xml=<<<XML
<?xml version='1.0'?>
<!DOCTYPE wddxPacket SYSTEM 'wddx_0100.dtd'>
<wddxPacket>
<var name="
XML;

$xml .= str_repeat('F',0x800000);

$xml .= <<<XML
">
</wddxPacket>
XML;
var_dump(wddx_deserialize($xml));
?>

Expected result:
----------------
NULL

Actual result:
--------------
$~ php7 --ini
Configuration File (php.ini) Path: /opt/php7/lib
Loaded Configuration File:         (none)
Scan for additional .ini files in: (none)
Additional .ini files parsed:      (none)

$~ php7 -v
PHP 7.2.0-dev (cli) (built: Sep 26 2016 15:29:49) ( NTS DEBUG )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.1.0-dev, Copyright (c) 1998-2016 Zend Technologies

$~ php7 wddx.php
NULL
[Mon Sep 26 15:43:39 2016]  Script:  '/opt/php7/bin/wddx.php'
/home/vps/git/php-src/ext/wddx/wddx.c(796) :  Freeing 0x00007f410d200000 (8388609 bytes), script=/opt/php7/bin/wddx.php
=== Total 1 memory leaks detected ===

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-09-26 10:17 UTC] tloi at fortinet dot com
My bad, the patch should null the pointer too:

--- a/ext/wddx/wddx.c
+++ b/ext/wddx/wddx.c
@@ -241,6 +241,10 @@ static int wddx_stack_destroy(wddx_stack *stack)
                }
                efree(stack->elements);
        }
+       if (stack->varname) {
+               efree(stack->varname);
+               stack->varname = NULL;
+       }
        return SUCCESS;
 }
 [2017-06-25 18:17 UTC] nikic@php.net
Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=604827b694aaf1ffcbd986043ba27642534f2ccf
Log: Fixed bug #73173
 [2017-06-25 18:17 UTC] nikic@php.net
-Status: Open +Status: Closed
 
PHP Copyright © 2001-2018 The PHP Group
All rights reserved.
Last updated: Sun Nov 18 02:01:26 2018 UTC