lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Sat, 26 Jul 2014 21:17:50 -0700
From:	Mark D Rustad <mrustad@...il.com>
To:	Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
Cc:	Herbert Xu <herbert@...dor.apana.org.au>,
	"David S. Miller" <davem@...emloft.net>,
	linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] crypto: crypto_user.c:  Cleaning up missing null-terminate in conjunction with strncpy

Rickard,

On Jul 26, 2014, at 7:15 AM, Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se> wrote:

> Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
> 
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
> ---
> crypto/crypto_user.c |   12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> index e2a34fe..09b465e 100644
> --- a/crypto/crypto_user.c
> +++ b/crypto/crypto_user.c
> @@ -77,7 +77,7 @@ static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg)
> {
> 	struct crypto_report_cipher rcipher;
> 
> -	strncpy(rcipher.type, "cipher", sizeof(rcipher.type));
> +	strlcpy(rcipher.type, "cipher", sizeof(rcipher.type));
> 
> 	rcipher.blocksize = alg->cra_blocksize;
> 	rcipher.min_keysize = alg->cra_cipher.cia_min_keysize;

This patch is an example of what I mentioned in my previous message. I figured I should go back and take a look at more of the patches. It looks to me like all of the changes in this patch create information leaks, because strlcpy only copies to the terminator, leaving the rest of the destination area in an otherwise uninitialized stack area alone. That structure is then copied whole into an skb and sent as a netlink message.

That will leak kernel information into userspace.

All of these patches need to be considered in regard to information leakage.

> @@ -96,7 +96,7 @@ static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg)
> {
> 	struct crypto_report_comp rcomp;
> 
> -	strncpy(rcomp.type, "compression", sizeof(rcomp.type));
> +	strlcpy(rcomp.type, "compression", sizeof(rcomp.type));
> 	if (nla_put(skb, CRYPTOCFGA_REPORT_COMPRESS,
> 		    sizeof(struct crypto_report_comp), &rcomp))
> 		goto nla_put_failure;
> @@ -109,10 +109,10 @@ nla_put_failure:
> static int crypto_report_one(struct crypto_alg *alg,
> 			     struct crypto_user_alg *ualg, struct sk_buff *skb)
> {
> -	strncpy(ualg->cru_name, alg->cra_name, sizeof(ualg->cru_name));
> -	strncpy(ualg->cru_driver_name, alg->cra_driver_name,
> +	strlcpy(ualg->cru_name, alg->cra_name, sizeof(ualg->cru_name));
> +	strlcpy(ualg->cru_driver_name, alg->cra_driver_name,
> 		sizeof(ualg->cru_driver_name));
> -	strncpy(ualg->cru_module_name, module_name(alg->cra_module),
> +	strlcpy(ualg->cru_module_name, module_name(alg->cra_module),
> 		sizeof(ualg->cru_module_name));
> 
> 	ualg->cru_type = 0;
> @@ -125,7 +125,7 @@ static int crypto_report_one(struct crypto_alg *alg,
> 	if (alg->cra_flags & CRYPTO_ALG_LARVAL) {
> 		struct crypto_report_larval rl;
> 
> -		strncpy(rl.type, "larval", sizeof(rl.type));
> +		strlcpy(rl.type, "larval", sizeof(rl.type));
> 		if (nla_put(skb, CRYPTOCFGA_REPORT_LARVAL,
> 			    sizeof(struct crypto_report_larval), &rl))
> 			goto nla_put_failure;

-- 
Mark Rustad, MRustad@...il.com


Download attachment "signature.asc" of type "application/pgp-signature" (842 bytes)

Powered by blists - more mailing lists