radareorg / ideas

4 stars 1 forks source link

rop search improvement #81

Open kamou opened 8 years ago

kamou commented 8 years ago

Hello,

I found some improvement that can be done for the rop search command, but I'd like to implement it myself. I just can't figure out how to get the size of the end_gadget, could you please help ?

Improvement explanation:

the "problem" is that the backward search starts from the instruction right before the end_gadget. but an end gadget like: 41ff14dc call qword [r12 + rbx*8] can become: ff14dc call qword [rsp + rbx*8]

Which is a very interesting gadget... So please let me do this improvement, I really want to contribute to this wonderful tool :)

I've tried replacing (in function r_core_search_rop):

if (i >= next) {                                            
    // We've exhausted the first end-gadget section,
    // move to the next one.
    if (r_list_get_n (end_list, 0)) {
        prev = i;
        free(end_gadget);
        end_gadget = (struct endlist_pair *)r_list_pop(end_list);
        next = end_gadget->instr_offset;
        i = next - ropdepth;
        if (i <0) i = 0;
    } else {
        break;
    }
}

with this:

if (i >= next + 1) { // <<<<<<<<<<<<<<<<<<< look here !
    // We've exhausted the first end-gadget section,
    // move to the next one.
    if (r_list_get_n (end_list, 0)) {
        prev = i;                                                
        free(end_gadget);
        end_gadget = (struct endlist_pair *)r_list_pop(end_list);
        next = end_gadget->instr_offset;
        i = next - ropdepth;
        if (i <0) i = 0;
    } else {
        break;
    }
}

and it worked fine for that particular example. But I guess it would be better to do it on the size of the end gadget minus one.

Again, please let me do it, I just need some help with the APIs, I don't know them well yet...

kamou commented 8 years ago

thinking again about it I think it shouldn't even be improved there... I guess the end_list should have contained that shortened instruction (ff14dc), don't know yet why it didn't.

radare commented 8 years ago

Maybe this should be related to search.align eval var

radare commented 8 years ago

Maybe @jpenalbae or @crowell can give you some pointers if needed :)

jpenalbae commented 8 years ago

I would say that the way to go is to keep searching backwards starting with the instruction right before the end_gadget. What you might want to improve is the end_gadget selection, but I would not touch the backwards disassembly part.

Anyway, what is really needed, is an speed improvement, instead of adding more end_gadgets to the list which actually slows down the process even more. Probably adding an evar like "rop.retonly = true" could be a good idea in order to limit end_gadgets just to c3 and see if that gives us a good speed improvement.

jvoisin commented 8 years ago

The backward disassembler is a bit messy, maybe it should be improved too.

XVilka commented 5 years ago

@kamou have you decided anything about this one?

XVilka commented 5 years ago

but I'd like to implement it myself.

@kamou how is it going by the way?

ret2libc commented 4 years ago

This issue has been moved from radareorg/radare2 to radareorg/ideas as we are trying to clean our backlog and this issue has probably been created a long while ago. This is an effort to help contributors understand what are the actionable items they can work on, prioritize issues better and help users find active/duplicated issues more easily. If this is not an enhancement/improvement/general idea but a bug, feel free to ask for re-transfer to main repo. Thanks for your understanding and contribution with this issue.