oers / reversatile

Reversatile: Reversi for Android
GNU General Public License v3.0
19 stars 6 forks source link

Cherry pick bug fixes in C code from panstromek/zebra #82

Closed panstromek closed 2 years ago

panstromek commented 2 years ago

When working on https://github.com/panstromek/zebra-rs, I found bunch of bugs in the original zebra by fuzzing them together.

I fixed them in my fork here https://github.com/panstromek/zebra, it'd make sense to cherry pick them to Reversatile. C code in Reversatile is not quite the same as in the original zebra, but there are not that many differences, so it shouldn't be difficult to port in most cases.

Some of them are quite nasty (some buffer oveflows) but mostly have low impact, because they are either triggered in some obscure conditions (invalid command line arguments) or don't cause any harm (off by one errors like reading one byte past the array length). That said, it's UB, so C makes no guarantees about any behaviour of the program 🤷‍♂️

oers commented 2 years ago

Could you provide a PR? My C skills suck :D

panstromek commented 2 years ago

I can look at that when I have some time, I just wanted to log it here so it's not forgotten :) (or if someone else would want to take a shot sooner)

More specifically, it's roughly this commit range from here (basically everything with prefix fix):

https://github.com/panstromek/zebra/commit/a2e4fef594b3deb00cd8acedc5d223b8bd5d29e3 .. https://github.com/panstromek/zebra/commit/9a5b789261bf32a65a2df134a8ccf7ba85c4df98

Most of the commits are quite trivial - adding u suffix to literals, bounds check, making arrays bigger, initialize variables etc.

panstromek commented 2 years ago

I should mention that some of them are also already fixed here, like the buffer overflow here https://github.com/oers/reversatile/commit/c095b9f1694352ecfbf4bc411d350b1af437fd4f (https://github.com/panstromek/zebra/commit/ecf160e7637eb9406804029f47192fad5d52aca3 in my fork)

oers commented 2 years ago

i started with the MR and now undo/redo is not working :)

oers commented 2 years ago

should work now :)

panstromek commented 2 years ago

Nice! Did you encounter any problem with porting the code? (except for undo/redo?)