> On May 26, 2020, at 15:10, Mattia Rizzolo <mat...@debian.org> wrote:
>
> * building the package shows this "scary" GCC warning:
> |In file included from /usr/include/string.h:495,
> | from cryptopass.c:19:
> |In function 'strncpy',
> | inlined from 'main' at cryptopass.c:200:9:
> |/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: warning:
> '__builtin___strncpy_chk' specified bound depends on the length of the source
> argument [-Wstringop-overflow=]
> | 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos
> (__dest));
> | |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> |cryptopass.c: In function 'main':
> |cryptopass.c:191:25: note: length computed here
> | 191 | passlenbuflen = strlen(argv[3]);
> | | ^~~~~~~~~~~~~~~
This prompted me to take a quick look at the source. There are multiple
trivially exploitable buffer overflows in this code. E.g.
src/cryptopass.c:147-149 [0]:
usernamelen = strlen(argv[1]);
memcpy(username, argv[1], usernamelen);
You could argue this program is only intended to receive input from a trusted
user, but is a user meant to comprehend that passing large command line
arguments results in memory corruption? Obviously everyone is free to develop
code how they like, but IMHO security packages should be using fuzz testing,
that would easily find this issue. AFAICT this code base has no test suite. I
would suggest adding one as well as fuzzing this code before exposing the
downstream public to it.
[0]:
https://github.com/basilgello/cryptopass/blob/master/src/cryptopass.c#L147-L149
