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 |
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) {
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 --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) {