| Message ID | 1470057550-58098-2-git-send-email-seth.forshee@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 01, 2016 at 08:19:10AM -0500, Seth Forshee wrote: > In general the handling of IMA/EVM xattrs is good, but I found > a few locations where either the xattr size or the value of the > type field in the xattr are not checked. Add a few simple checks > to these locations to prevent malformed or malicious xattrs from > causing problems. > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> Bump. I haven't seen any feedback on this patch, but I also don't think it's been applied anywhere, so I suspect it might have been forgotten. Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2016-08-29 at 13:31 -0500, Seth Forshee wrote: > On Mon, Aug 01, 2016 at 08:19:10AM -0500, Seth Forshee wrote: > > In general the handling of IMA/EVM xattrs is good, but I found > > a few locations where either the xattr size or the value of the > > type field in the xattr are not checked. Add a few simple checks > > to these locations to prevent malformed or malicious xattrs from > > causing problems. > > > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > > Bump. I haven't seen any feedback on this patch, but I also don't think > it's been applied anywhere, so I suspect it might have been forgotten. Thanks for the reminder. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 4304372b323f..106e855e2d9d 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -51,7 +51,7 @@ static bool init_keyring __initdata; int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, const char *digest, int digestlen) { - if (id >= INTEGRITY_KEYRING_MAX) + if (id >= INTEGRITY_KEYRING_MAX || siglen < 2) return -EINVAL; if (!keyring[id]) { diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index b9e26288d30c..35ab453ce861 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -145,6 +145,10 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, /* check value type */ switch (xattr_data->type) { case EVM_XATTR_HMAC: + if (xattr_len != sizeof(struct evm_ima_xattr_data)) { + evm_status = INTEGRITY_FAIL; + goto out; + } rc = evm_calc_hmac(dentry, xattr_name, xattr_value, xattr_value_len, calc.digest); if (rc) diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 1bcbc12e03d9..11a17073e8a2 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -130,6 +130,7 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len) { struct signature_v2_hdr *sig; + enum hash_algo ret; if (!xattr_value || xattr_len < 2) /* return default hash algo */ @@ -143,7 +144,9 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, return sig->hash_algo; break; case IMA_XATTR_DIGEST_NG: - return xattr_value->digest[0]; + ret = xattr_value->digest[0]; + if (ret < HASH_ALGO__LAST) + return ret; break; case IMA_XATTR_DIGEST: /* this is for backward compatibility */
In general the handling of IMA/EVM xattrs is good, but I found a few locations where either the xattr size or the value of the type field in the xattr are not checked. Add a few simple checks to these locations to prevent malformed or malicious xattrs from causing problems. Signed-off-by: Seth Forshee <seth.forshee@canonical.com> --- security/integrity/digsig.c | 2 +- security/integrity/evm/evm_main.c | 4 ++++ security/integrity/ima/ima_appraise.c | 5 ++++- 3 files changed, 9 insertions(+), 2 deletions(-)