snprintf() doesn't work the way you think it works

[kinnison]

Hi,

On line 69 of crypto-mcf.c you compare the result value of calling snprintf:

    if (s > SCRYPT_MCF_LEN)
        return 0;

Sadly this doesn't match the manpage for snprintf which states: Thus, a return value of size or more means that the output was truncated

You had a fix which increased the define for the MCF length by 1 which masked this issue, but this is likely the underlying cause of confusion surrounding test thirteen's failure mode before that fix.

[technion]

Many thanks for reporting this, I'm running tests across a fix right now.

The previous issue with failing test 12 was still related to the MCF being shorter than originally calculated - however this issue certainly did mask that.

For some reason I'm getting 3kb/s to Coverity, so give me a bit of time to complete the test run.

[kinnison]

Sorry for being so short with the original report, I was a tad frustrated, having spent ages with the version of libscrypt in Debian's Jessie trying to work out why I couldn't validate a password I had just hashed :-)

Thank you for responding so quickly.

[technion]

Sorry for being so short with the original report

Short in describing a short bug makes perfect sense.

Debian's Jessie

I don't know how closely they follow releases, but I'll be pushing a 0.21 version tag shortly.

To make sure we've got these length issues under control, I've more fully documented the total length calculation in the header file, where you can identify the earlier off by one count issue. I'll also round the size up for alignment reasons.

[kinnison]

If you want, before you push 0.21, you might want to document that libscrypt_check() mutates its inputs. Yes the signature of char* ought to warn me that might happen, but given how much software has non-const-correct APIs, I ignored that possibility to begin with and was bitten :-/

[technion]

Too late for the push, but I will get that documented.