nesbox / TIC-80

TIC-80 is a fantasy computer for making, playing and sharing tiny games.
https://tic80.com
MIT License
4.83k stars 460 forks source link

Fix tab-completion crash with load/save/etc. #2533

Closed aliceisjustplaying closed 1 month ago

aliceisjustplaying commented 1 month ago

Issue

When using tab completion with load/save in a folder with a lot of files, or really anything that'd result in an output longer than CONSOLE_BUFFER_SCREEN, TIC-80 would crash. This is due to data->options of TabCompleteData overflowing and not being null-terminated.

Minimal repro:

Create 100 blank .tic files:

#!/bin/bash
mkdir repro && cd repro || exit 1
for i in {001..100}; do
  touch "test-$i.tic"
done

Start TIC-80 with the repro folder as its base: ./build/bin/tic80 --fs ~/repro

Type "load" and it'll likely crash. maybe not at first, but after a few tries at most, definitely.

Notes

This was a hell of a bug to track down, but also a fun one that made me learn lldb and ASAN. I have some other PRs coming soon about that.

Future improvements

I have a few suggestions for future improvements and PRs:

String handling

This is a larger discussion, but it's worth considering either:

Curious of your thoughts about the potential improvements I've listed in the two sections above.


Fixes https://github.com/nesbox/TIC-80/issues/2448.

nesbox commented 1 month ago

I've created a separate issue for your suggested future improvements https://github.com/nesbox/TIC-80/issues/2540. Regarding string handling, I'm sure strlcat/strncat is the easiest way to make string handling safer :) Thank you 👍