search

joxeankoret / diaphora

Diaphora, the most advanced Free and Open Source program diffing tool.
http://diaphora.re
GNU Affero General Public License v3.0
3.58k stars 371 forks source link

Bug report #245

Closed HongThatCong closed 1 year ago

HongThatCong commented 1 year ago

Hi @joxeankoret I found some bugs in source files: diaphora.py and diaphora_ida.py

  1. diaphora.py Form https://github.com/joxeankoret/diaphora/blob/a369c7caf119a7d6ad8414772b1f18f1b244c795/diaphora.py#L1335 to https://github.com/joxeankoret/diaphora/blob/a369c7caf119a7d6ad8414772b1f18f1b244c795/diaphora.py#L1349 unreliable and partial variable still None, cause Exception. Need check None.

  2. diaphora_ida.py 2.1. import_instruction_level function: Line: https://github.com/joxeankoret/diaphora/blob/a369c7caf119a7d6ad8414772b1f18f1b244c795/diaphora_ida.py#L1365 Should be same as line https://github.com/joxeankoret/diaphora/blob/a369c7caf119a7d6ad8414772b1f18f1b244c795/diaphora_ida.py#L1387 Missing: "ins.operand_names operand_names" in sql statement

2.2. Line https://github.com/joxeankoret/diaphora/blob/a369c7caf119a7d6ad8414772b1f18f1b244c795/diaphora_ida.py#L1384 Should be same as line https://github.com/joxeankoret/diaphora/blob/a369c7caf119a7d6ad8414772b1f18f1b244c795/diaphora_ida.py#L1401

2.3. Need check operand_names is empty

Thanks you very much, @joxeankoret

joxeankoret commented 1 year ago

Hi! Unfortunately, it seems it's showing wrong lines right now for diaphora.py. Would you mind please updating it?

joxeankoret commented 1 year ago

I have (locally) fixed the problem with importing at instruction level. Thank you very much for reporting this! I will have to modify my testing suite to also test importing symbols.

UPDATE: I have just committed a fix for the part I can reproduce: 2f2c847c05292b192bedb06ce0019e290eb14d5b

gottfriedleibniz commented 1 year ago

The other issue being reported is where the unreliable (and maybe partial?) chooser can be None.

For example, follow: self.add_matches_from_cursor_ratio_max to unreliable.add_item, partial.add_item, and partial.add_item. Other uses of add_matches_from_cursor_ratio_max may require double-checking also.

joxeankoret commented 1 year ago

Aaah, I see. Already fixed locally, I will commit later (whenever I have time).

HongThatCong commented 1 year ago

In file diaphora_ida.py, I see there is a typo that has existed for a long time, from the first versions Function import_definitions type_name == "union" # == or = ????

joxeankoret commented 1 year ago

Mother of $deity. That is a bug that has been there for $deity knows how long... Thanks a lot for reporting!

UPDATE: The bug is there because the testing suite doesn't have support for verifying that importing was correct. This has been also the reason of yet another bug.

joxeankoret commented 1 year ago

Everything should be fixed with ebb3350ee9607705f46c9365fc74e20d93591f7a and 466c706e66cae53b85749b477a2cb0e3ea37f62e

0x79H commented 1 year ago

Everything should be fixed with ebb3350 and 466c706

AttributeError: 'NoneType' object has no attribute 'add_item' partial is None at https://github.com/joxeankoret/diaphora/blob/466c706e66cae53b85749b477a2cb0e3ea37f62e/diaphora.py#L1330 because partial = None https://github.com/joxeankoret/diaphora/blob/466c706e66cae53b85749b477a2cb0e3ea37f62e/diaphora.py#L1816 same at https://github.com/joxeankoret/diaphora/blob/466c706e66cae53b85749b477a2cb0e3ea37f62e/diaphora.py#L1878 I didn't read the code but I fix like ebb3350, it just work, but I don't know if it would break the code.

joxeankoret commented 1 year ago

Ah, you're right. The thing is that these problems are fixed in my development branch, which greatly differs from the current public one, I have to manually port all the changes and... I missed various. Thanks for reporting! I will fix it 'soon'.

joxeankoret commented 1 year ago

And done. Thank you very much for reporting!