arekinath / pivy

Tools for using PIV tokens (like Yubikeys) as an SSH agent, for encrypting data at rest, and more
193 stars 26 forks source link

Avoid overwriting slotid with stack garbage #40

Closed botovq closed 1 year ago

botovq commented 1 year ago

When the calculation of slotid was converted to piv_slotid_from_string() a piece of the previous strotul() parsing was left behind in f73d2a76 overwriting the just computed slotid with stack garbage.

botovq commented 1 year ago

There are a few more warnings, but as they aren't about actual bugs, I refrained from touching those.

piv-certs.c:2556:23: note: uninitialized use occurs here
        freezero(sig, siglen);
                      ^~~~~~
piv-certs.c:2508:9: note: remove the 'if' if its condition is always false
        if (rc != 1) {
        ^~~~~~~~~~~~~~
piv-certs.c:2494:30: note: initialize the variable 'siglen' to silence this warning
        size_t tbslen, siglen;
                             ^
                              = 0

pivy-box.c:185:13: warning: variable 'parsed' is uninitialized when used here [-Wuninitialized]
                        slotid = parsed;
                                 ^~~~~~
pivy-box.c:132:21: note: initialize the variable 'parsed' to silence this warning
        unsigned int parsed;
                           ^
                            = 0

pivy-agent.c:2065:7: warning: unused variable 'i' [-Wunused-variable]
        uint i;
             ^
pivy-agent.c:2066:8: warning: unused variable 'f' [-Wunused-variable]
        FILE *f;
              ^
arekinath commented 1 year ago

Thanks! That's an embarrassing one. I'll go through those other warnings, too. Looks like I need to build with clang more often between releases.