gnotclub / xst

st fork that uses Xresources and some pretty good patches
MIT License
536 stars 74 forks source link

Reproducible SIGSEGV with st.font_fallback #159

Open maybebyte opened 1 year ago

maybebyte commented 1 year ago

Hey, I've been working on writing an OpenBSD port for this and noticed this compiler warning (please bear in mind throughout this issue that I'm not a C developer):

In file included from x.c:243:
./xst.c:76:34: warning: expression which evaluates to zero treated as a null pointer constant of type 'char *' [-Wnon-literal-null-conversion]
                                font2[endchar + count + 1] = '\0';

I looked at the relevant code:

config.def.h line 9:

static char *font2[] = {
/*  "Inconsolata for Powerline:pixelsize=12:antialias=true:autohint=true", */
/*  "Hack Nerd Font Mono:pixelsize=11:antialias=true:autohint=true", */
};

xst.c line 65:

        XRESOURCE_LOAD_META("font_fallback") {
            int count = 0, endchar = fonts_count = sizeof(font2) / sizeof(*font2);
            for (int i = 0; ret.addr[i]; i++) if (ret.addr[i] == ',') count++;
            if (count > 0)
            {
                for (int i = 0; i <= count; i++)
                {
                    if (i == 0) font2[endchar + i] = strtok(ret.addr, ",");
                    else                font2[endchar + i] = strtok(NULL, ",");
                    fonts_count++;
                }
                font2[endchar + count + 1] = '\0';
            } else if (ret.addr) {
                font2[endchar] = ret.addr;
                fonts_count++;
            }
        }

What I realized is that you can reliably cause xst to dump a core file by passing a comma (either alone or as a trailing comma) to st.font_fallback because it increments count:

$ grep st.font_fallback ~/.Xresources
st.font_fallback: ,

$ xst
Segmentation fault (core dumped)

$ egdb -q xst xst.core
Reading symbols from xst...
[New process 195719]
Core was generated by `xst'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00000ec5e2d55653 in xloadsparefonts () at x.c:1173
1173            if (**fp == '-')
(gdb) bt
#0  0x00000ec5e2d55653 in xloadsparefonts () at x.c:1173
#1  0x00000ec5e2d5480d in xinit (cols=80, rows=24) at x.c:1333
#2  0x00000ec5e2d542d4 in main (argc=<optimized out>, argv=<optimized out>) at x.c:2759
(gdb) quit

It seems like it leads into undefined behavior. I'm unsure what the best way to address this would be, but wanted to bring it to your attention all the same.