Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Memory leak in libxml encoding handling #17223

Open
chongwick opened this issue Dec 20, 2024 · 8 comments · May be fixed by #17254
Open

Memory leak in libxml encoding handling #17223

chongwick opened this issue Dec 20, 2024 · 8 comments · May be fixed by #17254
Labels
Bug Extension: libxml Status: Fixed in Third Party Library Status: Verified

Comments

@chongwick
Copy link

chongwick commented Dec 20, 2024 •
edited
Loading

Description

The following code:

<?php
$malicious_document = new DOMDocument();
$malicious_document->__construct(str_repeat(chr(223), 65537) . str_repeat(chr(8), 17) . str_repeat(chr(133), 257), str_repeat(chr(62), 257));
$malicious_document-> save(str_repeat("%s%x%n", 0x100), 0.5880082824695007);

Resulted in this output:

==3938990==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x14e1d1ff8887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x14e1d1cb85ba  (/lib/x86_64-linux-gnu/libxml2.so.2+0x3d5ba)

Indirect leak of 32640 byte(s) in 1 object(s) allocated from:
    #0 0x14e1d1ff8887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x14e1d192f7e6 in __gconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x2a7e6)
    #2 0x14e1d192f2b7 in iconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x2a2b7)
    #3 0x61200002c5bf  (<unknown module>)

Indirect leak of 32640 byte(s) in 1 object(s) allocated from:
    #0 0x14e1d1ff8887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x14e1d192f7e6 in __gconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x2a7e6)
    #2 0x14e1d192f2b7 in iconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x2a2b7)
    #3 0x602000002b6f  (<unknown module>)

Indirect leak of 416 byte(s) in 2 object(s) allocated from:
    #0 0x14e1d1ff8887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x14e1d193a9d6  (/lib/x86_64-linux-gnu/libc.so.6+0x359d6)

Indirect leak of 258 byte(s) in 1 object(s) allocated from:
    #0 0x14e1d1ff8887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x14e1d1d5a942  (/lib/x86_64-linux-gnu/libxml2.so.2+0xdf942)

Indirect leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x14e1d1ff8887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x14e1d192f76c in __gconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x2a76c)
    #2 0x14e1d192f2b7 in iconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x2a2b7)
    #3 0x61200002c5bf  (<unknown module>)

Indirect leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x14e1d1ff8887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x14e1d192f76c in __gconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x2a76c)
    #2 0x14e1d192f2b7 in iconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x2a2b7)
    #3 0x602000002b6f  (<unknown module>)

SUMMARY: AddressSanitizer: 66234 byte(s) leaked in 8 allocation(s).

PHP Version

8.5-dev

Operating System

No response
@devnexen
Copy link
Member

devnexen commented Dec 20, 2024 •
edited
Loading

Hi and thanks for your report however I got to admit I m rarely able to reproduce your issues recently. Is there any info missing ? nvm leak sanitizer on mac is only partially supported, I can reproduce on linux.

@chongwick
Copy link
Author

I'll make sure to be more consistent with OS info, etc.

@nielsdos
Copy link
Member

@chongwick This is not a PHP bug, but a libxml bug. There is a memory leak here: https://github.com/GNOME/libxml2/blame/0dd910e82bf74cdebcabe1a576f6178e0092cb5d/xmlsave.c#L2830, and a quick and dirty test of adding an xmlCharEncCloseFunc(handler); call prior to the return fixes this. I suggest you report this upstream to libxml.

@devnexen
Copy link
Member

ah true definitively there is no php involvement in this case, should have read better the report..

@chongwick
Copy link
Author

https://gitlab.gnome.org/GNOME/libxml2/-/issues/831

@nielsdos
Copy link
Member

Thanks, since this is now tracked upstream, I believe we can close this.

@nielsdos nielsdos closed this as not planned Won't fix, can't repro, duplicate, stale Dec 21, 2024
@nwellnhof
Copy link

It's a bug in both libxml2 and PHP. The libxml2 fix is here: https://gitlab.gnome.org/GNOME/libxml2/-/commit/b3871dd138ad68a22ef315809bbe4b071090711b

In PHP, you have to make php_libxml_output_buffer_create_filename free the encoder argument even if an error occurs.

On another note, I think you could avoid the whole xmlOutputBufferCreateFilenameDefault hack by using xmlSaveToIO instead of xmlSaveFormatFileEnc (and xmlOutputBufferCreateIO instead of xmlOutputBufferCreateFilename).

@nielsdos
Copy link
Member

@nwellnhof Thanks! I'll look into it tomorrow or Monday as it's already late in the evening here

@nielsdos nielsdos reopened this Dec 21, 2024
@nielsdos nielsdos changed the title memory leak in DOMDocument Memory leak in DOMDocument encoding handling Dec 24, 2024
@nielsdos nielsdos changed the title Memory leak in DOMDocument encoding handling Memory leak in libxml encoding handling Dec 24, 2024
nielsdos added a commit to nielsdos/php-src that referenced this issue Dec 24, 2024
@nielsdos
Fix phpGH-17223: Memory leak in libxml encoding handling
5a32924 
This was a bug in both libxml and PHP.
We follow up with the same change as done in GNOME/libxml@b3871dd138.

Changing away from `xmlOutputBufferCreateFilenameDefault` is not
possible yet because this is a stable branch and would break BC.
@nielsdos nielsdos linked a pull request Dec 24, 2024 that will close this issue
Fix GH-17223: Memory leak in libxml encoding handling #17254
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Extension: libxml Status: Fixed in Third Party Library Status: Verified
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix GH-17223: Memory leak in libxml encoding handling nielsdos/php-src
4 participants
@nwellnhof @devnexen @nielsdos @chongwick