phc-discussions - Argon2 code review

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGiyFdc3OTmR+GT8C4U+8QuSoO1zoZGR8EE7igp7eJ9e86=PBg@mail.gmail.com>
Date: Fri, 11 Sep 2015 14:27:35 +0000
From: Jean-Philippe Aumasson <jeanphilippe.aumasson@...il.com>
To: "discussions@...sword-hashing.net" <discussions@...sword-hashing.net>, 
	Dmitry Khovratovich <khovratovich@...il.com>
Subject: Argon2 code review

Quickly looked at the Argon2 code at https://github.com/khovratovich/Argon2.
Some very basic comments below:

* use more standard directory structure, e.g. s/Source/src

* enable highest warning level in the makefile

* better use `fopen_s`  than `fopen` on windows

* fix warnings and errors when compiling with clang, e.g.
Source/Core/argon2-core.cpp:272:27: warning: comparison of 0 > unsigned
expression is always false [-Wtautological-compare]
        if (MIN_AD_LENGTH > context->adlen) {
            ~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
Source/Test/argon2-test.cpp:69:40: error: use of undeclared identifier
'__rdtscp'
                        start_cycles = __rdtscp(&ui2);


* fix relevant issues found by cppcheck:
[Blake2/blake2b.cpp]: (information) Too many #ifdef configurations -
cppcheck only checks 12 of 13 configurations. Use --force to check all
configurations.
[Core/argon2-core.h:65]: (error) Buffer is accessed out of bounds: v
[Core/argon2-core.h:73]: (error) Buffer is accessed out of bounds: v
[Core/argon2-core.h:85]: (error) Buffer is accessed out of bounds: v
[Core/argon2-core.cpp:1]: (information) Skipping configuration 'memset_s'
since the value of 'memset_s' is unknown. Use -D if you want to check it.
You can use -U to skip it explicitly.
[Core/argon2-opt-core.cpp:176] -> [Core/argon2-opt-core.cpp:177]: (warning)
Possible null pointer dereference: instance - otherwise it is redundant to
check it against null.
[Core/argon2-opt-core.cpp:238] -> [Core/argon2-opt-core.cpp:239]: (warning)
Possible null pointer dereference: instance - otherwise it is redundant to
check it against null.
[Core/argon2-ref-core.cpp:93] -> [Core/argon2-ref-core.cpp:94]: (warning)
Possible null pointer dereference: instance - otherwise it is redundant to
check it against null.
[Core/argon2-ref-core.cpp:154] -> [Core/argon2-ref-core.cpp:155]: (warning)
Possible null pointer dereference: instance - otherwise it is redundant to
check it against null.
[Core/kat.cpp:93]: (error) Resource leak: fp
[Core/kat.cpp:107]: (error) Resource leak: fp
[Core/kat.cpp:125]: (error) Resource leak: fp
[Test/argon2-test.cpp:88]: (warning) %d in format string (no. 1) requires
'int' but the argument type is 'unsigned int'.
[Test/argon2-test.cpp:355]: (warning) %d in format string (no. 3) requires
'int' but the argument type is 'unsigned int'.
(information) Cppcheck cannot find all the include files (use
--check-config for details)

Content of type "text/html" skipped