Closed nename0 closed 3 years ago
Hi!
Thank you for PR. I'll review it next week.
Seems like Ubuntu 20 broke the python2 Github Actions.
I'll also fix CI during review. Can you check that tests run well on your PC?
Yeah, im working on a test that shoud fail with the old version...
I just downgraded to ubuntu 18 (obviously not a proper fix) to showcase this:
So here is the new test failing on CI using https://github.com/JonathanSalwan/ROPgadget/pull/170/commits/b6b8710bbdd9e56dfac509c0e430cdef3f7f77ec Here is the test failing with the exception not caught using https://github.com/JonathanSalwan/ROPgadget/pull/170/commits/8cd2bb7bb4fa560ca0c40f651eb6af8debe4dcf9 And the last run is successful.
Excellent! Thank you for PR) Downgrading to ubuntu 18.04 is absolutely ok.
Is was digging around to find out, why badbytes ranges always gave my 0 gadgets. And I found this python3 compatibility bug caused by this line: https://github.com/JonathanSalwan/ROPgadget/blob/7c5d4cffc58dfa5e3ce45bdc5e6690122c70f8f3/ropgadget/options.py#L135 As
chr
returns a string in python3 andbbytes
should be a list of single character byte-strings, we later get this exception:However this was not detected because of this
raise
-except
goto
-reimplementation you use in many places: https://github.com/JonathanSalwan/ROPgadget/blob/7c5d4cffc58dfa5e3ce45bdc5e6690122c70f8f3/ropgadget/options.py#L142-L148 As you can see by this example, this construct hides bugs and should at least be protected by using a specific Exception-Type. Therefore I refactored it to afor
-else
construct, which fits better in this case.