On Mon, 2018-11-26 at 07:39 +0300, Vitaly Chikunov wrote:
> Commit ae1319eeabd6 ("Remove hardcoding of SHA1 in EVM signatures")
> introduces overflow of 20 byte buffer on the stack while calculating evm
> hash. Also, invalid hash length is passed to the underlying verification
> function. This prevents any non-SHA1 hashes from being properly
> validated using evmctl.
>
> Signed-off-by: Vitaly Chikunov <vt@xxxxxxxxxxxx>
Thanks! To prevent this sort of bug from recurring, it would be nice
if the maximum digest size would be defined once and used.
Mimi
> ---
> src/evmctl.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 1b46d58..94d7ab1 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -55,6 +55,7 @@
> #include <keyutils.h>
> #include <ctype.h>
> #include <termios.h>
> +#include <assert.h>
>
> #include <openssl/sha.h>
> #include <openssl/pem.h>
> @@ -760,13 +761,15 @@ static int cmd_sign_evm(struct command *cmd)
>
> static int verify_evm(const char *file)
> {
> - unsigned char hash[20];
> + unsigned char hash[64];
> unsigned char sig[1024];
> + int mdlen;
> int len;
>
> - len = calc_evm_hash(file, hash);
> - if (len <= 1)
> - return len;
> + mdlen = calc_evm_hash(file, hash);
> + assert(mdlen <= sizeof(hash));
> + if (mdlen <= 1)
> + return mdlen;
>
> len = lgetxattr(file, "security.evm", sig, sizeof(sig));
> if (len < 0) {
> @@ -779,7 +782,7 @@ static int verify_evm(const char *file)
> return -1;
> }
>
> - return verify_hash(file, hash, sizeof(hash), sig + 1, len - 1);
> + return verify_hash(file, hash, mdlen, sig + 1, len - 1);
> }
>
> static int cmd_verify_evm(struct command *cmd)
