geohot / qira

QEMU Interactive Runtime Analyser
MIT License
3.94k stars 471 forks source link

Incorrect pmaps revealed by IDA plugin #118

Open nedwill opened 9 years ago

nedwill commented 9 years ago

I'm beginning work on the IDA plugin, and I noticed that sometimes selecting data in IDA does not select the data back in QIRA. This is because QIRA filters any setdaddr for an address it believes is an instruction.

In ida.js:

if (dat[0] == "setdaddr") {
  if (get_data_type(dat[1]) != "datainstruction") {
    update_dview(dat[1]);
  }
}

But get_data_type looks this up in the pmap that ultimately comes from the qiradb. The pmaps are stored by page, masked off with PAGE_MASK. If any instructions from that page are executed, the entire page is marked with PAGE_INSTRUCTION.

Trace.cpp:

#define PAGE_MASK 0xFFFFFFFFFFFFF000LL
...
if (type == 'I') {
  ...
  pages_[c->address & PAGE_MASK] |= PAGE_INSTRUCTION;
}

But sometimes code and data are in the same page. See double_link:

vagrant@vagrant-ubuntu-trusty-64:/vagrant/qira$ readelf -S tests_manual/double_link
There are 30 section headers, starting at offset 0x1150:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
...
  [13] .text             PROGBITS        080483f0 0003f0 0001a2 00  AX  0   0 16
  [14] .fini             PROGBITS        08048594 000594 000014 00  AX  0   0  4
  [15] .rodata           PROGBITS        080485a8 0005a8 000013 00   A  0   0  4
  [16] .eh_frame_hdr     PROGBITS        080485bc 0005bc 00002c 00   A  0   0  4
  [17] .eh_frame         PROGBITS        080485e8 0005e8 0000b0 00   A  0   0  4
  [18] .init_array       INIT_ARRAY      08049f00 000f00 000004 00  WA  0   0  4
  [19] .fini_array       FINI_ARRAY      08049f04 000f04 000004 00  WA  0   0  4

According the pmap, only .init_array and .fini_array are data, even though .rodata and eh_frame* are data too. Either I'm misunderstanding something here or we can do better by having static be responsible for informing the UI about the sections. This could lead to a more accurate haddrline as well.

geohot commented 9 years ago

Hmm, so I did that because of page mappings, but feel free to make it more granular(add sections). I agree it'll improve the haddrline, and you can use static to track sections.

Now that we know shellcode works(it reconstructs the Instruction from the dynamic memory store on view, not the worst solution), this actually looks more like a bug than dynamic static.