linux-kernel - Re: [PATCH] crypto: ablkcipher.c: Cleaning up missing null-terminate in conjunction with strncpy

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <22C5E3AE-403A-48B7-86EC-F6E4AC42D4A5@gmail.com>
Date:	Sat, 26 Jul 2014 21:58:25 -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: ablkcipher.c:  Cleaning up missing null-terminate in conjunction with strncpy

Rickard,

On Jul 26, 2014, at 7:09 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/ablkcipher.c |    8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
> index 40886c4..e446eef 100644
> --- a/crypto/ablkcipher.c
> +++ b/crypto/ablkcipher.c
> @@ -384,8 +384,8 @@ static int crypto_ablkcipher_report(struct sk_buff *skb, struct crypto_alg *alg)
> {
> 	struct crypto_report_blkcipher rblkcipher;
> 
> -	strncpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type));
> -	strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<default>",
> +	strlcpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type));
> +	strlcpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<default>",
> 		sizeof(rblkcipher.geniv));
> 
> 	rblkcipher.blocksize = alg->cra_blocksize;
> @@ -465,8 +465,8 @@ static int crypto_givcipher_report(struct sk_buff *skb, struct crypto_alg *alg)
> {
> 	struct crypto_report_blkcipher rblkcipher;
> 
> -	strncpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type));
> -	strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<built-in>",
> +	strlcpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type));
> +	strlcpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<built-in>",
> 		sizeof(rblkcipher.geniv));
> 
> 	rblkcipher.blocksize = alg->cra_blocksize;

It looks like all of these patches in the crypto area are introducing information leaks, including the one that first made me worry and respond. There were no bugs here (unless someone were to introduce a crypto method with a name longer than 63 characters), because stncpy fills the remainder of the destination with 0.

There are times when strncpy is the right function.

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


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