Fix memory leaks in linux heap flow #4426

giridharprasath · 2024-04-10T01:59:52Z

SQUASH ME

Your checklist for this pull request

I've read the guidelines for contributing to this repository
I made sure to follow the project's coding style
I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
I've added tests that prove my fix is effective or that my feature works (if possible)
I've updated the rizin book with the relevant information (if needed)

Detailed description

Fix memory leak in bin_elf and fix a leak in dmht
Add support for aarch64 tcache parsing
...

Test plan

Tested on ARM64 build

...

Closing issues

...

librz/bin/bin.c

giridharprasath · 2024-04-12T09:15:52Z

@wargio @imbillow

Please review(one testcase failure) and lmk if we have to change this testcase output?

[XX] db/formats/pe/imports_tinyW7 PE: resolve reloc name by ordinal from sdb
RZ_NOPLUGINS=1 /usr/bin/rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -eflirt.sigdb.load.system=false -eflirt.sigdb.load.home=false -N -Qc 'ir
ii
' bins/pe/imports_tinyW7.exe
-- stdout
--- expected
+++ actual
@@ -4,5 +4,5 @@
 0x800004f4 0x00000234 SET_32 msvcrt_Ordinal_1268
 nth  vaddr      bind type lib      name         
 ------------------------------------------------
-284  ---------- NONE FUNC kernel32 FindAtomW
+284  0x00401048 NONE FUNC kernel32 Ordinal_284
 1268 0x00401034 NONE FUNC msvcrt   Ordinal_1268

imbillow · 2024-04-15T16:28:39Z

@giridharprasath There's something wrong with this test.

[XX] db/formats/pe/imports_tinyW7 PE: resolve reloc name by ordinal from sdb
RZ_NOPLUGINS=1 /usr/bin/rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -eflirt.sigdb.load.system=false -eflirt.sigdb.load.home=false -N -Qc 'ir
ii
' bins/pe/imports_tinyW7.exe
-- stdout
--- expected
+++ actual
@@ -4,5 +4,5 @@
 0x800004f4 0x00000234 SET_32 msvcrt_Ordinal_1268
 nth  vaddr      bind type lib      name         
 ------------------------------------------------
-284  ---------- NONE FUNC kernel32 FindAtomW
+284  0x00401048 NONE FUNC kernel32 Ordinal_284
 1268 0x00401034 NONE FUNC msvcrt   Ordinal_1268

librz/bin/p/bin_mach0.c

librz/bin/bin.c

XVilka · 2024-04-16T15:53:49Z

Interesting, FindAtomW is 283 ordinal, not 284, see librz/bin/d/dll/kernel32.sdb.txt. So the previous output was a bug it seems?

librz/core/linux_heap_glibc.c

giridharprasath · 2024-04-19T01:58:43Z

Ghidra's imports:

giridharprasath · 2024-04-19T02:06:34Z

Debug binary with dmht command:
Before fix:

After fix:

XVilka · 2024-04-19T10:41:05Z

librz/core/linux_heap_glibc.c

 		if (tcache_guess < map->addr || tcache_guess > map->addr_end) {
 			continue;
 		}

+#if __aarch64__


In the future, parsing should not depend on the architecture Rizin was built on.

pelijah · 2024-04-24T18:20:52Z

librz/bin/bin.c

+	rz_bin_import_free(reloc->import);
+	rz_bin_symbol_free(reloc->symbol);


These changes are not related to heap parsing and not tested well. Reloc related changes cause UAF and must be reverted.

Ok. @giridharprasath could you please send a PR reverting those? @pelijah do you have an example of the test file where it happens?

Sure @XVilka, will send them. I have tested this with ELF and PE binaries. The fix caused UAF in mach binaries, I have fixed there as well. @pelijah Please add where it is reproducible.

COFF files and some LE libs

There was some reason why relocs refer to original imports/symbols. I have concerns that breaking this unwritten rule may cause side effects.

* Fix memory leaks in debug path * Fix memory leaks in mach and mdmp format * Update the fix for mach * Add support for aarch64 tcache parsing * Test case fix * Comment update --------- Co-authored-by: Giridhar Prasath R <giridh1337@gmail.com>

…org#4426)" This reverts commit 6e0a256.

* Fix memory leaks in debug path * Fix memory leaks in mach and mdmp format * Update the fix for mach * Add support for aarch64 tcache parsing * Test case fix * Comment update --------- Co-authored-by: Giridhar Prasath R <giridh1337@gmail.com>

…org#4426)" This reverts commit 081b3d9.

giridharprasath requested review from ret2libc, wargio, XVilka, kazarmy and thestr4ng3r as code owners April 10, 2024 01:59

github-actions bot added RzBin RzCore labels Apr 10, 2024

giridharprasath marked this pull request as draft April 10, 2024 02:13

wargio reviewed Apr 10, 2024

View reviewed changes

librz/bin/bin.c Outdated Show resolved Hide resolved

giridharprasath force-pushed the dev branch from 4810051 to 9364c57 Compare April 12, 2024 07:42

github-actions bot added PE Mach-O labels Apr 12, 2024

giridharprasath marked this pull request as ready for review April 13, 2024 02:55

wargio reviewed Apr 16, 2024

View reviewed changes

librz/bin/p/bin_mach0.c Outdated Show resolved Hide resolved

wargio reviewed Apr 16, 2024

View reviewed changes

librz/bin/bin.c Outdated Show resolved Hide resolved

giridharprasath force-pushed the dev branch from 5444d7b to 082e3ea Compare April 18, 2024 21:48

giridharprasath requested a review from yossizap as a code owner April 18, 2024 21:48

github-actions bot added the RzDebug label Apr 18, 2024

github-advanced-security bot found potential problems Apr 18, 2024

View reviewed changes

librz/core/linux_heap_glibc.c Fixed Show fixed Hide fixed

github-actions bot added the rz-test label Apr 19, 2024

XVilka approved these changes Apr 19, 2024

View reviewed changes

XVilka requested a review from wargio April 19, 2024 11:07

Giridhar Prasath R added 3 commits April 20, 2024 02:29

Fix memory leaks in debug path

3a50ca0

Fix memory leaks in mach and mdmp format

28de2c6

Update the fix for mach

54a8ac6

Giridhar Prasath R added 4 commits April 20, 2024 02:29

Add support for aarch64 tcache parsing

7cd6254

Linter fixes

6b65e44

Test case fix

6641e3c

Comment update

6f3479a

giridharprasath force-pushed the dev branch from 892c31c to 6f3479a Compare April 20, 2024 06:29

XVilka approved these changes Apr 20, 2024

View reviewed changes

Rot127 approved these changes Apr 20, 2024

View reviewed changes

XVilka merged commit 0551f25 into rizinorg:dev Apr 21, 2024
44 checks passed

pelijah reviewed Apr 24, 2024

View reviewed changes

giridharprasath mentioned this pull request Apr 26, 2024

Revert memory leak fix for relocs #4463

Merged

5 tasks

kazarmy added a commit to kazarmy/rizin that referenced this pull request May 2, 2024

Revert "[TO BE REVERTED] Fix memory leaks in linux heap parser (rizin…

ff0addc

…org#4426)" This reverts commit 6e0a256.

kazarmy added a commit to kazarmy/rizin that referenced this pull request May 3, 2024

Revert "[TO BE REVERTED] Fix memory leaks in linux heap parser (rizin…

0cd5349

…org#4426)" This reverts commit 081b3d9.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix memory leaks in linux heap flow #4426

Fix memory leaks in linux heap flow #4426

giridharprasath commented Apr 10, 2024 •

edited by XVilka

giridharprasath commented Apr 12, 2024

imbillow commented Apr 15, 2024

XVilka commented Apr 16, 2024

giridharprasath commented Apr 19, 2024

giridharprasath commented Apr 19, 2024

XVilka Apr 19, 2024

pelijah Apr 24, 2024

pelijah Apr 24, 2024

XVilka Apr 25, 2024

giridharprasath Apr 25, 2024

pelijah Apr 25, 2024

pelijah Apr 25, 2024

		rz_bin_import_free(reloc->import);
		rz_bin_symbol_free(reloc->symbol);

Fix memory leaks in linux heap flow #4426

Fix memory leaks in linux heap flow #4426

Conversation

giridharprasath commented Apr 10, 2024 • edited by XVilka

SQUASH ME

giridharprasath commented Apr 12, 2024

imbillow commented Apr 15, 2024

XVilka commented Apr 16, 2024

giridharprasath commented Apr 19, 2024

giridharprasath commented Apr 19, 2024

XVilka Apr 19, 2024

Choose a reason for hiding this comment

pelijah Apr 24, 2024

Choose a reason for hiding this comment

pelijah Apr 24, 2024

Choose a reason for hiding this comment

XVilka Apr 25, 2024

Choose a reason for hiding this comment

giridharprasath Apr 25, 2024

Choose a reason for hiding this comment

pelijah Apr 25, 2024

Choose a reason for hiding this comment

pelijah Apr 25, 2024

Choose a reason for hiding this comment

giridharprasath commented Apr 10, 2024 •

edited by XVilka