Bug#961429: RFS: cryptopass/1.0.0-1 [ITP] -- CLI utility for generating long, unguessable passwords.

[Matthew Fernandez <matthew.fernandez@gmail.com>]

> On May 26, 2020, at 15:10, Mattia Rizzolo <mattia@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