[v1,1/5] ima-evm-utils: Fix null dereference from file2bin to memcpy
diff mbox series

Message ID 20190712212833.29280-1-vt@altlinux.org
State New
Headers show
Series
  • [v1,1/5] ima-evm-utils: Fix null dereference from file2bin to memcpy
Related show

Commit Message

Vitaly Chikunov July 12, 2019, 9:28 p.m. UTC
file2bin() may return NULL, which is set to tmp, which is passed to
memcpy. Add explicit check for it. CID 229904.
---
 src/evmctl.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Mimi Zohar July 15, 2019, 7:08 p.m. UTC | #1
On Sat, 2019-07-13 at 00:28 +0300, Vitaly Chikunov wrote:
> file2bin() may return NULL, which is set to tmp, which is passed to
> memcpy. Add explicit check for it. CID 229904.

Maybe move the CID to a "Fixes" tag with an indication of the CID
origin.

> ---
>  src/evmctl.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index a6d07c9..39bc3d9 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -821,7 +821,15 @@ static int verify_ima(const char *file)
>  	if (sigfile) {
>  		void *tmp = file2bin(file, "sig", &len);
> 
> -		assert(len <= sizeof(sig));

Thanks for removing the "assert".  It would stop the measurement list
verification or walking a file system in the middle.

> +		if (!tmp) {
> +			log_err("Failed reading: %s\n", file);
> +			return -1;
> +		}
> +		if (len > sizeof(sig)) {
> +			log_err("File is too big: %s\n", file);

We're reading the file signature file.  Perhaps say,"File signature is
...".

> +			free(tmp);
> +			return -1;
> +		}
>  		memcpy(sig, tmp, len);
>  		free(tmp);
>  	} else {

Patch
diff mbox series

diff --git a/src/evmctl.c b/src/evmctl.c
index a6d07c9..39bc3d9 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -821,7 +821,15 @@  static int verify_ima(const char *file)
 	if (sigfile) {
 		void *tmp = file2bin(file, "sig", &len);
 
-		assert(len <= sizeof(sig));
+		if (!tmp) {
+			log_err("Failed reading: %s\n", file);
+			return -1;
+		}
+		if (len > sizeof(sig)) {
+			log_err("File is too big: %s\n", file);
+			free(tmp);
+			return -1;
+		}
 		memcpy(sig, tmp, len);
 		free(tmp);
 	} else {