diff mbox series

[v1,2/5] ima-evm-utils: Fix possible strcpy overflow

Message ID 20190712212833.29280-2-vt@altlinux.org (mailing list archive)
State New, archived
Headers show
Series [v1,1/5] ima-evm-utils: Fix null dereference from file2bin to memcpy | expand

Commit Message

Vitaly Chikunov July 12, 2019, 9:28 p.m. UTC 
caps_str is passed from command line but copied into fixed-size buffer.
CID 229895.
---
 src/evmctl.c | 3 ++-
 1 file changed, 2 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:
> caps_str is passed from command line but copied into fixed-size buffer.
> CID 229895.
> ---
>  src/evmctl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 39bc3d9..e07cff4 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -409,8 +409,9 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
>  		} else if (!strcmp(*xattrname, XATTR_NAME_CAPS) && (hmac_flags & HMAC_FLAG_CAPS_SET)) {
>  			if (!caps_str)
>  				continue;
> -			strcpy(xattr_value, caps_str);
>  			err = strlen(caps_str);
> +			assert(err < sizeof(xattr_value));

"calc_evm_hash" can be called while walking and labeling, or
verifying, a file system.  We probably don't want to abruptly end it.
 Maybe emit an error message and return and error?


> +			strcpy(xattr_value, caps_str);
>  		} else {
>  			err = lgetxattr(file, *xattrname, xattr_value, sizeof(xattr_value));
>  			if (err < 0) {
Vitaly Chikunov July 15, 2019, 8:05 p.m. UTC | #2 
On Mon, Jul 15, 2019 at 03:08:55PM -0400, Mimi Zohar wrote:
> On Sat, 2019-07-13 at 00:28 +0300, Vitaly Chikunov wrote:
> > caps_str is passed from command line but copied into fixed-size buffer.
> > CID 229895.
> > ---
> >  src/evmctl.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/evmctl.c b/src/evmctl.c
> > index 39bc3d9..e07cff4 100644
> > --- a/src/evmctl.c
> > +++ b/src/evmctl.c
> > @@ -409,8 +409,9 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
> >  		} else if (!strcmp(*xattrname, XATTR_NAME_CAPS) && (hmac_flags & HMAC_FLAG_CAPS_SET)) {
> >  			if (!caps_str)
> >  				continue;
> > -			strcpy(xattr_value, caps_str);
> >  			err = strlen(caps_str);
> > +			assert(err < sizeof(xattr_value));
> 
> "calc_evm_hash" can be called while walking and labeling, or
> verifying, a file system.  We probably don't want to abruptly end it.
>  Maybe emit an error message and return and error?

Ok. I will also add similar checks for selinux_str and ima_str.

Thanks,

> 
> 
> > +			strcpy(xattr_value, caps_str);
> >  		} else {
> >  			err = lgetxattr(file, *xattrname, xattr_value, sizeof(xattr_value));
> >  			if (err < 0) {
diff mbox series

Patch

diff --git a/src/evmctl.c b/src/evmctl.c
index 39bc3d9..e07cff4 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -409,8 +409,9 @@  static int calc_evm_hash(const char *file, unsigned char *hash)
 		} else if (!strcmp(*xattrname, XATTR_NAME_CAPS) && (hmac_flags & HMAC_FLAG_CAPS_SET)) {
 			if (!caps_str)
 				continue;
-			strcpy(xattr_value, caps_str);
 			err = strlen(caps_str);
+			assert(err < sizeof(xattr_value));
+			strcpy(xattr_value, caps_str);
 		} else {
 			err = lgetxattr(file, *xattrname, xattr_value, sizeof(xattr_value));
 			if (err < 0) {