2014-07-26 14:07:52

by Rickard Strandqvist

[permalink] [raw]

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

---

Replacing strncpy with strlcpy to avoid strings that lacks null terminate.

Signed-off-by: Rickard Strandqvist <[email protected]>
---
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;
--
1.7.10.4

2014-07-27 04:58:29

by Mark D Rustad

[permalink] [raw]

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 <[email protected]> wrote:

> Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
>
> Signed-off-by: Rickard Strandqvist <[email protected]>
> ---
> 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, [email protected]

---

Attachments:

signature.asc (841.00 B)
Message signed with OpenPGP using GPGMail