telescope-browser / telescope

browser for the small internet
https://telescope-browser.org
ISC License
47 stars 1 forks source link

Fix crash at strlcpy on macos #4

Closed sikmir closed 2 years ago

sikmir commented 2 years ago
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib          0x00007fff70fddb66 __pthread_kill + 10
1   libsystem_pthread.dylib         0x00007fff711a8080 pthread_kill + 333
2   libsystem_c.dylib               0x00007fff70f391ae abort + 127
3   libsystem_c.dylib               0x00007fff70f39321 abort_report_np + 177
4   libsystem_c.dylib               0x00007fff70f5dbf5 __chk_fail + 48
5   libsystem_c.dylib               0x00007fff70f5dbc5 __chk_fail_overflow + 16
6   libsystem_c.dylib               0x00007fff70f5dcb0 __strlcpy_chk + 85
7   telescope                       0x0000000100c644ae load_gemini_url + 106
8   telescope                       0x0000000100c6384e do_load_url + 442
9   telescope                       0x0000000100c635a1 load_url + 64
10  telescope                       0x0000000100c65ded ui_main_loop + 19
11  telescope                       0x0000000100c63f77 main + 1038
12  libdyld.dylib                   0x00007fff70e8d015 start + 1
omar-polo commented 2 years ago

Ouch, that's a bad copy-paste error! Thanks!

I'll roll a bugfix release as soon as I manage to (likely tomorrow morning)

omar-polo commented 2 years ago

slightly changing my mind, I've accumulated enough stuff to tag v0.6 (I wanted to include more, but anyway), so it'll take me a bit still (need to recheck everything, ensure documentation is clear etc)

Just for curiosity, what was the URL like? (I assume it was something along the lines of gemini://example.com:1234567890123456.../.) I'd like to not only truncate the string (as done here) but also detect invalid port numbers as so and inform the user appropriately. Having some real-world example would be nice :)

sikmir commented 2 years ago

Just for curiosity, what was the URL like? (I assume it was something along the lines of gemini://example.com:1234567890123456.../.)

Nothing special, it crashed with gemini://gemini.omarpolo.com.

omar-polo commented 2 years ago

Oh, right, now I see. __strlcpy_chk checks that dstlen < len and aborts otherwise. Yesterday I misread the stacktrace and confused it with a segmentation fault ^^" apologies

(furthermore, just for the record, tab->url.port is defined as char [6] and req.port as char [16] so that can't possibly overflow, but I need to change the sizes for consistency)

Thanks again :)

omar-polo commented 2 years ago

last thing I promise :)

It's OK if I add the following to the ChangeLog:

2021-11-25  Nikolay Korotkiy <email-address-taken-from-git-here>

    * telescope.c (load_gemini_url): fix macOs crash on `__strlcpy_chk' due to wrong lengths

or do you prefer Nikolay Korotkiy <sikmir@github>? (or something else entirely)

I'd like to highlight important improvements/bugfixes on the ChangeLog file (that then is used to generate the per-release changelog message)

sikmir commented 2 years ago

No problem:) Yes, sikmir@gmail.com is OK.