Not analyzing an x64 SEH handler after RaiseException #2247

segevfiner · 2022-01-24T21:35:20Z

Work environment

Questions	Answers
OS/arch/bits (mandatory)	Windows x64 64
File format of the file you reverse (mandatory)	PE
Architecture/bits of the file (mandatory)	x64
`rizin -v` full output, not truncated (mandatory)	rizin 0.4.0-git @ windows-x86-64 commit: `8282cee`, build: 24/01/2022__20:53:48.12

Expected behavior

I expect rizin to correctly identify x64 SEH handlers after RaiseException and add their relevant basic blocks/ranges to the function.

I encountered this while trying to look at OutputDebugStringA in kernelbase.dll.

Actual behavior

The function is truncated on RaiseException. This happens whether I enable analysis.trycatch or not, which I assume is needed to enable analyzing exception handlers?

Steps to reproduce the behavior

#include <Windows.h>
#include <cstring>
#include <cstdio>

#define MSG "Hello, World!\n"

int main()
{
	ULONG_PTR args[2];
	args[0] = std::strlen(MSG) + 1;
	args[1] = reinterpret_cast<ULONG_PTR>(MSG);
	
	__try {
		RaiseException(DBG_PRINTEXCEPTION_C, 0, _countof(args), args);
	} __except (GetExceptionCode() == DBG_PRINTEXCEPTION_C ? EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH) {
		std::printf("caught\n");
	}

	return 0;
}

Binary: main.zip

Additional Logs, screenshots, source code, configuration dump, ...

The text was updated successfully, but these errors were encountered:

segevfiner · 2022-01-26T10:30:38Z

Also of note is that RaiseException is marked as noreturn, yet I think it does return if a debugger swallows the exception. But I'm not sure.

segevfiner · 2022-01-26T11:27:04Z

Doing tn- RaiseException makes analysis continue and see the jmp and the code following it. But it still seems to miss the exception handler even with analysis.trycatch enabled. I think there is also another function that contains the expression in the __except clause which could technically be detected and named.

segevfiner · 2022-01-26T13:55:59Z

Looks like for the binary I supplied, this is never entered:

rizin/librz/analysis/fcn.c

Lines 790 to 815 in 56c6e83

    
           if (analysis->opt.trycatch) { 
        
           	const char *name = analysis->coreb.getName(analysis->coreb.core, at); 
        
           	if (name) { 
        
           		if (rz_str_startswith(name, "try.") && rz_str_endswith(name, ".from")) { 
        
           			char *handle = strdup(name); 
        
           			// handle = rz_str_replace (handle, ".from", ".to", 0); 
        
           			ut64 from_addr = analysis->coreb.numGet(analysis->coreb.core, handle); 
        
           			handle = rz_str_replace(handle, ".from", ".catch", 0); 
        
           			ut64 handle_addr = analysis->coreb.numGet(analysis->coreb.core, handle); 
        
           			bb->jump = at + oplen; 
        
           			if (from_addr != bb->addr) { 
        
           				bb->fail = handle_addr; 
        
           				ret = analyze_function_locally(analysis, fcn, handle_addr); 
        
           				eprintf("(%s) 0x%08" PFMT64x "\n", handle, handle_addr); 
        
           				if (bb->size == 0) { 
        
           					rz_analysis_function_remove_block(fcn, bb); 
        
           				} 
        
           				rz_analysis_block_unref(bb); 
        
           				bb = fcn_append_basic_block(analysis, fcn, addr); 
        
           				if (!bb) { 
        
           					gotoBeach(RZ_ANALYSIS_RET_ERROR); 
        
           				} 
        
           			} 
        
           		} 
        
           	} 
        
           }

Even with enabling analysis.trycatch which is obviously required and setting tn- RaiseException it looks like it never reaches a point where it will consider the try.

segevfiner · 2022-01-27T17:36:15Z

Argh. Apparently the try flags are not created by default even when you enable analysis.trycatch. I had to do .iw. So the following sequence for the given binary:

tn- RaiseException
.iw
e analysis.trycatch=true
aaaa

Produces:

[(try.2.14000120c.catch) 0x14000133asym. and entry0 (aa)
WARNING: rz_analysis_block_update_hash: assertion 'block' failed (line 701)
(try.1.14000120c.catch) 0x14000133a
WARNING: rz_analysis_block_update_hash: assertion 'block' failed (line 701)
(try.0.140001000.catch) 0x140001050
WARNING: rz_analysis_block_update_hash: assertion 'block' failed (line 701)
[x] Analyze all flags starting with sym. and entry0 (aa)
[(try.3.1400014b0.catch) 0x140001541
WARNING: rz_analysis_block_update_hash: assertion 'block' failed (line 701)
(try.9.140011c80.catch) 0x140011cc0
WARNING: rz_analysis_block_update_hash: assertion 'block' failed (line 701)
(try.5.140004ffc.catch) 0x1400050b7
WARNING: rz_analysis_block_update_hash: assertion 'block' failed (line 701)
[x] Analyze function calls (aac)
[x] Analyze len bytes of instructions for references (aar)
[x] Check for classes
[x] Type matching analysis for all functions (aaft)
[x] Propagate noreturn information
[x] Use -AA or aaaa to perform additional experimental analysis.
[x] Finding function preludes
[x] Enable constraint types analysis for variables
[x] Applied 0 FLIRT signatures via sigdb

Which has this suspicious assertions, though I'm not yet sure what it means. And the disassembly:

Which is weird, analysis didn't continue properly after analyzing the from/to pair (Maybe due to the assertion?). Running VV causes it to prompt Rendering graph...Function was modified. Reanalyze? (Y/n) (Are the hashes that weren't updated what's causing this?) which then results in a complete disassembly as it reanalyzes the function:

I wasn't able to trigger reanalysis using other commands though.

Guess this needs some work...

segevfiner · 2022-01-28T14:47:54Z

This change got it working for the test binary:

diff --git a/librz/analysis/fcn.c b/librz/analysis/fcn.c
index 35afbbc7a..df76ba572 100644
--- a/librz/analysis/fcn.c
+++ b/librz/analysis/fcn.c
@@ -805,7 +805,7 @@ static RzAnalysisBBEndCause run_basic_block_analysis(RzAnalysisTaskItem *item, R
 							rz_analysis_function_remove_block(fcn, bb);
 						}
 						rz_analysis_block_unref(bb);
-						bb = fcn_append_basic_block(analysis, fcn, addr);
+						bb = fcn_append_basic_block(analysis, fcn, bb->jump);
 						if (!bb) {
 							gotoBeach(RZ_ANALYSIS_RET_ERROR);
 						}

(Can submit a PR with this if wanted, note that this code also has a stray debugging eprintf in it).

Using the following command sequence:

tn- RaiseException
.iw
e analysis.trycatch=true
aaaa

(Might want to consider unsetting noreturn on RaiseException by default (And maybe RtlRaiseException?, or others?))

But I still wasn't able to get it to analyze OutputDebugStringA in KernelBase.dll completely. It doesn't seem like there is a try there according to iw, Which is strange cause it does look like an exception is handled there to go to the case where there is no debugger, otherwise it doesn't seem to make much sense, not sure if there might be some issue parsing the SEH x64 table, or there is some other trickery going on to reach that code. It also sadly doesn't continue analysis to the end of the function stopping after at the jmp after RaiseException for some reason....

P.S. I'm still getting a constant "Function was modified. Reanalyze?" while entering visual graph on that function, not sure why... Also it might make sense for it to link one opcode before to instead of the from to the exception handler bb. And it still doesn't really account for the SEH filter expression.

segevfiner · 2022-01-28T15:32:39Z

Yep. The .pdata or exception directory for kernelbase.dll seems to have way more entries than what iw prints, so it is likely that rizin doesn't understand all the formats of unwind info that exist there.

Like the code in

rizin/librz/bin/p/bin_pe64.c

Line 361 in 6af80b3

static RzList *trycatch(RzBinFile *bf) {

is pretty weird, like it might only tries to parse C++ try catchor SEH or something?

ret2libc · 2022-01-31T09:20:01Z

@GustavoLCR did you implement this?

segevfiner · 2022-02-13T19:12:18Z

With latest changes:

It does seem to analyze more exception handlers, not sure if it's all of them (Didn't compare the output manually).
I think it still links the exception handler to the try flag rather than the opcode before the catch flag.
tn- RaiseException is still required.
You still need to manually run .iw & e analysis.trycatch=true.
Haven't checked how it handles the filter yet.

P.S. I got Linear size differs too much from the bbsum, please use pdr instead. when doing this and trying to pdf OutputDebugStringA in KernelBase.dll. With this sequence of commands. Also without those commands rizin seems to have stopped analyzing some functions from the sample binary after hitting a jmp opcode

XVilka · 2022-02-14T03:13:25Z

We will need to add a better test for this as well.

Your checklist for this pull request - [x] I've read the guidelines for contributing to this repository - [x] 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 Bin - Load exception information by default Analysis - Make analyzing exception scopes the default - Analyze all exception sources as functions - Do not hack basic blocks flow to add try/catch information Graph - Add an edge pointing to the catch block for each basic block inside the try scope - Add configuration graph.trycatch to control if graphing the exception blocks is enabled Problems 8ff06e9 Is basically copy-pasted logic from block.c This can make graphs a lot noisier Cutter will have to implement its own version of the logic if it wants to copy rizin in showing the connection between the blocks and its catch blocks I didn't add tests yet Test plan Open bins\pe\microsoft_seh_tests\x64\xcpt4.exe for example Optional: idp to load debug info for binary aaa Optional: .iw to show try catch scopes as flags s main Enter visual graph mode, make sure all edges, lines are painted correctly, and that there area now new edges pointing each basic block to its catch block if it is inside a try scope Open file from rizinorg#2247 aaa s main Enter visual graph mode, make sure issue is fixed Closing issues Closes rizinorg#2247

ret2libc added the PE label Jan 25, 2022

XVilka added the test-required label Jan 25, 2022

GustavoLCR mentioned this issue Feb 13, 2022

PE x64 SEH fixes #2310

Merged

5 tasks

XVilka added the C++ C++ binaries label Feb 14, 2022

XVilka assigned GustavoLCR Jun 27, 2022

GustavoLCR linked a pull request Jul 30, 2022 that will close this issue

Fixes #2247 - Improve try/catch analysis #2854

Open

5 tasks

j-barnak linked a pull request Feb 28, 2024 that will close this issue

Try Catch Improvement #4305

Draft

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Not analyzing an x64 SEH handler after RaiseException #2247

Not analyzing an x64 SEH handler after RaiseException #2247

segevfiner commented Jan 24, 2022 •

edited by ret2libc

Loading

segevfiner commented Jan 26, 2022

segevfiner commented Jan 26, 2022

segevfiner commented Jan 26, 2022 •

edited

Loading

segevfiner commented Jan 27, 2022

segevfiner commented Jan 28, 2022 •

edited

Loading

segevfiner commented Jan 28, 2022 •

edited

Loading

ret2libc commented Jan 31, 2022

segevfiner commented Feb 13, 2022 •

edited

Loading

XVilka commented Feb 14, 2022

Not analyzing an x64 SEH handler after RaiseException #2247

Not analyzing an x64 SEH handler after RaiseException #2247

Comments

segevfiner commented Jan 24, 2022 • edited by ret2libc Loading

Work environment

Expected behavior

Actual behavior

Steps to reproduce the behavior

Additional Logs, screenshots, source code, configuration dump, ...

segevfiner commented Jan 26, 2022

segevfiner commented Jan 26, 2022

segevfiner commented Jan 26, 2022 • edited Loading

segevfiner commented Jan 27, 2022

segevfiner commented Jan 28, 2022 • edited Loading

segevfiner commented Jan 28, 2022 • edited Loading

ret2libc commented Jan 31, 2022

segevfiner commented Feb 13, 2022 • edited Loading

XVilka commented Feb 14, 2022

segevfiner commented Jan 24, 2022 •

edited by ret2libc

Loading

segevfiner commented Jan 26, 2022 •

edited

Loading

segevfiner commented Jan 28, 2022 •

edited

Loading

segevfiner commented Jan 28, 2022 •

edited

Loading

segevfiner commented Feb 13, 2022 •

edited

Loading