From f9db44b2ad38f1e6f9ff88359da550b5ec0e5d74 Mon Sep 17 00:00:00 2001 From: Mathias Krause Date: Tue, 5 Feb 2013 18:19:13 +0100 Subject: crypto: user - fix info leaks in report API Three errors resulting in kernel memory disclosure: 1/ The structures used for the netlink based crypto algorithm report API are located on the stack. As snprintf() does not fill the remainder of the buffer with null bytes, those stack bytes will be disclosed to users of the API. Switch to strncpy() to fix this. 2/ crypto_report_one() does not initialize all field of struct crypto_user_alg. Fix this to fix the heap info leak. 3/ For the module name we should copy only as many bytes as module_name() returns -- not as much as the destination buffer could hold. But the current code does not and therefore copies random data from behind the end of the module name, as the module name is always shorter than CRYPTO_MAX_ALG_NAME. Also switch to use strncpy() to copy the algorithm's name and driver_name. They are strings, after all. Signed-off-by: Mathias Krause Cc: Steffen Klassert Signed-off-by: Herbert Xu --- crypto/crypto_user.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'crypto/crypto_user.c') diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c index 35d700a9..f6d9baf7 100644 --- a/crypto/crypto_user.c +++ b/crypto/crypto_user.c @@ -75,7 +75,7 @@ static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg) { struct crypto_report_cipher rcipher; - snprintf(rcipher.type, CRYPTO_MAX_ALG_NAME, "%s", "cipher"); + strncpy(rcipher.type, "cipher", sizeof(rcipher.type)); rcipher.blocksize = alg->cra_blocksize; rcipher.min_keysize = alg->cra_cipher.cia_min_keysize; @@ -94,8 +94,7 @@ static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg) { struct crypto_report_comp rcomp; - snprintf(rcomp.type, CRYPTO_MAX_ALG_NAME, "%s", "compression"); - + strncpy(rcomp.type, "compression", sizeof(rcomp.type)); if (nla_put(skb, CRYPTOCFGA_REPORT_COMPRESS, sizeof(struct crypto_report_comp), &rcomp)) goto nla_put_failure; @@ -108,12 +107,14 @@ nla_put_failure: static int crypto_report_one(struct crypto_alg *alg, struct crypto_user_alg *ualg, struct sk_buff *skb) { - memcpy(&ualg->cru_name, &alg->cra_name, sizeof(ualg->cru_name)); - memcpy(&ualg->cru_driver_name, &alg->cra_driver_name, - sizeof(ualg->cru_driver_name)); - memcpy(&ualg->cru_module_name, module_name(alg->cra_module), - CRYPTO_MAX_ALG_NAME); - + strncpy(ualg->cru_name, alg->cra_name, sizeof(ualg->cru_name)); + strncpy(ualg->cru_driver_name, alg->cra_driver_name, + sizeof(ualg->cru_driver_name)); + strncpy(ualg->cru_module_name, module_name(alg->cra_module), + sizeof(ualg->cru_module_name)); + + ualg->cru_type = 0; + ualg->cru_mask = 0; ualg->cru_flags = alg->cra_flags; ualg->cru_refcnt = atomic_read(&alg->cra_refcnt); @@ -122,8 +123,7 @@ static int crypto_report_one(struct crypto_alg *alg, if (alg->cra_flags & CRYPTO_ALG_LARVAL) { struct crypto_report_larval rl; - snprintf(rl.type, CRYPTO_MAX_ALG_NAME, "%s", "larval"); - + strncpy(rl.type, "larval", sizeof(rl.type)); if (nla_put(skb, CRYPTOCFGA_REPORT_LARVAL, sizeof(struct crypto_report_larval), &rl)) goto nla_put_failure; -- cgit v1.2.3 From 50540e6eba0e59c3a5e4551b9aa7868df4f7bd8d Mon Sep 17 00:00:00 2001 From: Mathias Krause Date: Tue, 5 Feb 2013 18:19:14 +0100 Subject: crypto: user - fix empty string test in report API The current test for empty strings fails because it is testing the address of a field, not a pointer. So the test will always be true. Test the first character in the string to not be null instead. Signed-off-by: Mathias Krause Cc: Steffen Klassert Signed-off-by: Herbert Xu --- crypto/crypto_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crypto/crypto_user.c') diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c index f6d9baf7..423a2670 100644 --- a/crypto/crypto_user.c +++ b/crypto/crypto_user.c @@ -196,7 +196,7 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh, struct crypto_dump_info info; int err; - if (!p->cru_driver_name) + if (!p->cru_driver_name[0]) return -EINVAL; alg = crypto_alg_match(p, 1); -- cgit v1.2.3 From 25633c1e43f8d2a9343a14bd019a05fad5615af5 Mon Sep 17 00:00:00 2001 From: Mathias Krause Date: Tue, 5 Feb 2013 18:19:15 +0100 Subject: crypto: user - ensure user supplied strings are nul-terminated To avoid misuse, ensure cru_name and cru_driver_name are always nul-terminated strings. Signed-off-by: Mathias Krause Signed-off-by: Herbert Xu --- crypto/crypto_user.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'crypto/crypto_user.c') diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c index 423a2670..dfd511fb 100644 --- a/crypto/crypto_user.c +++ b/crypto/crypto_user.c @@ -30,6 +30,8 @@ #include "internal.h" +#define null_terminated(x) (strnlen(x, sizeof(x)) < sizeof(x)) + static DEFINE_MUTEX(crypto_cfg_mutex); /* The crypto netlink socket */ @@ -196,6 +198,9 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh, struct crypto_dump_info info; int err; + if (!null_terminated(p->cru_name) || !null_terminated(p->cru_driver_name)) + return -EINVAL; + if (!p->cru_driver_name[0]) return -EINVAL; @@ -260,6 +265,9 @@ static int crypto_update_alg(struct sk_buff *skb, struct nlmsghdr *nlh, struct nlattr *priority = attrs[CRYPTOCFGA_PRIORITY_VAL]; LIST_HEAD(list); + if (!null_terminated(p->cru_name) || !null_terminated(p->cru_driver_name)) + return -EINVAL; + if (priority && !strlen(p->cru_driver_name)) return -EINVAL; @@ -287,6 +295,9 @@ static int crypto_del_alg(struct sk_buff *skb, struct nlmsghdr *nlh, struct crypto_alg *alg; struct crypto_user_alg *p = nlmsg_data(nlh); + if (!null_terminated(p->cru_name) || !null_terminated(p->cru_driver_name)) + return -EINVAL; + alg = crypto_alg_match(p, 1); if (!alg) return -ENOENT; @@ -368,6 +379,9 @@ static int crypto_add_alg(struct sk_buff *skb, struct nlmsghdr *nlh, struct crypto_user_alg *p = nlmsg_data(nlh); struct nlattr *priority = attrs[CRYPTOCFGA_PRIORITY_VAL]; + if (!null_terminated(p->cru_name) || !null_terminated(p->cru_driver_name)) + return -EINVAL; + if (strlen(p->cru_driver_name)) exact = 1; -- cgit v1.2.3