lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150929125548.GA15621@bolet.org> Date: Tue, 29 Sep 2015 14:55:48 +0200 From: Thomas Pornin <pornin@...et.org> To: discussions@...sword-hashing.net Subject: Re: [PHC] Specification of a modular crypt format (2) On Tue, Sep 29, 2015 at 03:05:59AM +0300, Alexander Cherepanov wrote: > AIUI the decoder tries to be strict. Then perhaps it's worth to > check that 'str' doesn't start with ' ' or '-' here and try to catch > overflows. Strictly speaking, the decoder tries to be strict when strictness is cheap. If we want a strict decoder of numerical values, strtoul() won't be enough; we would need something like this: /* ------------------------------------------------------------------- */ #include <limits.h> /* * Decode decimal integer from 'str'; the value is written in '*v'. * Returned value is a pointer to the next non-decimal character in the * string. If there is no digit at all, or the value encoding is not * minimal (extra leading zeros), or the value does not fit in an * 'unsigned long', then NULL is returned. */ static const char * decode_decimal(const char *str, unsigned long *v) { const char *orig; unsigned long acc; orig = str; acc = 0; for (orig = str;; str ++) { int c; c = *str; if (c < '0' || c > '9') { break; } c -= '0'; if (acc > (ULONG_MAX / 10)) { return NULL; } acc *= 10; if ((unsigned long)c > (ULONG_MAX - acc)) { return NULL; } acc += (unsigned long)c; } if (str == orig || (*orig == '0' && str != (orig + 1))) { return NULL; } *v = acc; return str; } /* ------------------------------------------------------------------- */ (This code is completely untested; I just wrote it in this email.) I am not sure this is worth the effort. It is not very complex to do (less than 40 lines of C code). > > if (pp->m < 1 || (pp->m >> 30) > 3) { > > Why not just compare with a constant here? This is to avoid a spurious warning. If I write this: if (pp->m < 1 || pp->m > 0xFFFFFFFF) then the compiler, on a 32-bit machine, will complain that the comparison result is "always true". I don't want spurious warnings: they train developers to ignore warnings. > > if (pp->p < 1 || pp->p > 64) { > > 255 here? Oh yes. Indeed. I fixed the specification, but forgot to change the code accordingly. Thanks ! --Thomas
Powered by blists - more mailing lists