SEGV error in sixel_encoder_setopt

[shinibufa]

Hello, Libsixel developers! We recently ran some fuzz testing on img2sixel 1.8.6 and encountered a SEGV bug.

Command To Reproduce the bug:

./img2sixel --outfile

Environment

• OS: Ubuntu 20.04
• gcc 9.4.0
• img2sixel 1.8.6

ASAN Report

================================================================= ==956668==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f338fb204c0 bp 0x610000000040 sp 0x7ffd8a0ab120 T0) ==956668==The signal is caused by a READ memory access. ==956668==Hint: address points to the zero page. #0 0x7f338fb204c0 in sixel_encoder_setopt (/lib/x86_64-linux-gnu/libsixel.so.1+0x3e4c0) #1 0x4ce64b in main /home/root/sp/Dataset/Libsixel/libsixel-1.8.6/converters/img2sixel.c:423:22 #2 0x7f338f69c082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16 #3 0x41d3fd in _start (/home/root/sp/Dataset/Libsixel/libsixel_aflpp/install/bin/img2sixel+0x41d3fd)

AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV (/lib/x86_64-linux-gnu/libsixel.so.1+0x3e4c0) in sixel_encoder_setopt ==956668==ABORTING

Many Thanks.

[t-bltg]

See https://github.com/saitoha/libsixel/issues/154.

[j4james]

The problem is that the sixel_encoder_setopt function gets passed a NULL value when the filename is omitted from the --outfile option. We can fix that with a NULL check in sixel_encoder_setopt, but I don't think that's actually the right solution. All we really need to do is mark the outfile option as having a required argument, and the validation will then be handled automatically.

This patch should fix it:

diff --git a/converters/img2sixel.c b/converters/img2sixel.c
index 708a6f1..17e1567 100644
--- a/converters/img2sixel.c
+++ b/converters/img2sixel.c
@@ -337,7 +337,7 @@ main(int argc, char *argv[])
     char const *optstring = "o:78ORp:m:eb:Id:f:s:c:w:h:r:q:kil:t:ugvSn:PE:B:C:DVH";
 #ifdef HAVE_GETOPT_LONG
     struct option long_options[] = {
-        {"outfile",          no_argument,        &long_opt, 'o'},
+        {"outfile",          required_argument,  &long_opt, 'o'},
         {"7bit-mode",        no_argument,        &long_opt, '7'},
         {"8bit-mode",        no_argument,        &long_opt, '8'},
         {"gri-limit",        no_argument,        &long_opt, 'R'},

[saitoha]

@shinibufa @j4james Thank you very much. I believe the reason I hadn't noticed this issue until now is that the test suite executed by make check did not include any invocations using long options. I feel that we should add tests for the other long options as well.