Not analyzing an x64 SEH handler after RaiseException

[segevfiner]

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: 8282cee287abdbf8664a0b2540bad2b5ea5b819d, 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, ...

[segevfiner]

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]

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]

Looks like for the binary I supplied, this is never entered: https://github.com/rizinorg/rizin/blob/56c6e8384b540ee8379e993eb41f02fbd93e5fdf/librz/analysis/fcn.c#L790-L815

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]

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]

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]

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 https://github.com/rizinorg/rizin/blob/6af80b33a79b65cfbaf735a00c4e6b796b8b9933/librz/bin/p/bin_pe64.c#L361 is pretty weird, like it might only tries to parse C++ try catchor SEH or something?

[ret2libc]

@GustavoLCR did you implement this?

[segevfiner]

With latest changes:

1. It does seem to analyze more exception handlers, not sure if it's all of them (Didn't compare the output manually).
2. I think it still links the exception handler to the try flag rather than the opcode before the catch flag.
3. tn- RaiseException is still required.
4. You still need to manually run .iw & e analysis.trycatch=true.
5. 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]

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