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.

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

Use AddVectoredExceptionHandler to register exception handlers #403

Merged
merged 26 commits into from
Dec 16, 2021

Conversation

tokatoka
Copy link
Member

@tokatoka tokatoka commented Dec 4, 2021

resolves #372
We should not use SetUnhandledExceptionFilter, it searches through the stack for exception handler, but possibly frida Stalker breaks it.
this pr uses AddVectoredExceptionHandler instead

f58e6a0

@tokatoka
Copy link
Member Author

tokatoka commented Dec 4, 2021

This one depends on changes included in #391
so I made a branch from it

@tokatoka tokatoka changed the title Exception fix Use AddVectoredExceptionHandler to register exception handlers Dec 4, 2021
@tokatoka tokatoka marked this pull request as draft December 5, 2021 00:05
@tokatoka
Copy link
Member Author

tokatoka commented Dec 5, 2021

Does the commit 4d082cd on this branch solves the issue for you too? @expend20 🙂
I tested this with cpp code, and also tested the original source code you pasted in the issue #372 with pageheap

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>

extern "C" __declspec(dllexport) size_t
    FuzzMeOOBR(const char *data, unsigned int len)
{

    char sdata[1024];
    memcpy(sdata, data, sizeof(sdata) < len ? sizeof(sdata) : len);

    if (len >= 8 
            && *(uint8_t *)(data + 0) == '1'
            && *(uint8_t *)(data + 1) == '3'
            && *(uint8_t *)(data + 2) == '3'
            && *(uint8_t *)(data + 3) == '7'
            ) {
                char *p = (char *)0;
                *p = 0;
    }
    return 0;
}

@tokatoka tokatoka marked this pull request as ready for review December 5, 2021 00:26
@tokatoka
Copy link
Member Author

tokatoka commented Dec 5, 2021

The stackoverflow issue is not fixed yet.
The RIP is transfered our exception handlers, which is good, but

overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Stack overflow!Crashed with STATUS_STACK_OVERFLOW
Child crashed!
Crashed with STATUS_ACCESS_VIOLATION

I observe double crash.
STATUS_STACK_OVERFLOW and STATUS_ACCESS_VIOLATION
I feel this is what is happening here https://peteronprogramming.wordpress.com/2016/08/10/crashes-you-cant-handle-easily-2-stack-overflows-on-windows/

@tokatoka
Copy link
Member Author

tokatoka commented Dec 5, 2021

I feel we need to handle this special case like here @domenukk for windows

if child_status == 137 {

My dump file says the second exception STATUS_ACCESS_VIOLATION is triggered here


possibly it happens because we need to allocate stacks even after the stack mem is close to zero.

but still we have no way to salvage the input that triggered the stackoverflow, unless we save as much stack memory as possible inside the crash handler.

@tokatoka
Copy link
Member Author

tokatoka commented Dec 5, 2021

image
My ida says this is where fire() jumps to...
so it's clear we run out of stack again inside the exception handler

@tokatoka
Copy link
Member Author

they allocate this size of buffer on the stack
https://github.com/Frommi/miniz_oxide/blob/6da05dd599b438fff2a05e3c2552cbdcba4cc5a7/miniz_oxide/src/deflate/buffer.rs#L8
and that's why we need 0x10000 bytes (64 * 1024)
I think it's fine for us to reserve 0x20000 bytes here.

@domenukk
Copy link
Member

Good work

@domenukk domenukk merged commit 79f9bcd into main Dec 16, 2021
@tokatoka tokatoka deleted the exception_fix branch December 21, 2021 14:09
khang06 pushed a commit to khang06/LibAFL that referenced this pull request Oct 11, 2022
…usplus#403)

* add

* unix fix

* unsafe positions

* another unsafe!

* ignore

* ignore

* make changes back

* fix

* fix

* fmt

* exception fix

* fix

* bug fix

* fmt

* fix things messed up during merge

* stack overflow fix

* fix

* fix

* fix

Co-authored-by: Andrea Fioraldi <andreafioraldi@gmail.com>
Co-authored-by: Dominik Maier <domenukk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

frida_libpng crashes on Windows with pageheap enabled instead of reporting a crash
3 participants