[PATCH] Fix potential overlap dest buffer

[PATCH] Fix potential overlap dest buffer

[Nigel Croxon]

To meet requirements of Common Criteria certification vulnerablility
assessment. Static code analysis has been run and found the following
error.  Overlapping_buffer: The source buffer potentially overlaps
with the destination buffer, which results in undefined
behavior for "memcpy".

The change is to use memmove instead of memcpy.

Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
---
 sha1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sha1.c b/sha1.c
index 11be7045..89b32f46 100644
--- a/sha1.c
+++ b/sha1.c
@@ -258,7 +258,7 @@ sha1_process_bytes (const void *buffer, size_t len, struct sha1_ctx *ctx)
 	{
 	  sha1_process_block (ctx->buffer, 64, ctx);
 	  left_over -= 64;
-	  memcpy (ctx->buffer, &ctx->buffer[16], left_over);
+	  memmove (ctx->buffer, &ctx->buffer[16], left_over);
 	}
       ctx->buflen = left_over;
     }
-- 
2.29.2

Re: [PATCH] Fix potential overlap dest buffer

[NeilBrown]

On Tue, 17 Aug 2021, Nigel Croxon wrote:
> To meet requirements of Common Criteria certification vulnerablility
> assessment. Static code analysis has been run and found the following
> error.  Overlapping_buffer: The source buffer potentially overlaps
> with the destination buffer, which results in undefined
> behavior for "memcpy".
> 
> The change is to use memmove instead of memcpy.
> 
> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>

Reviewed-by: NeilBrown <neilb@suse.de>

Thanks,
NeilBrown


> ---
>  sha1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sha1.c b/sha1.c
> index 11be7045..89b32f46 100644
> --- a/sha1.c
> +++ b/sha1.c
> @@ -258,7 +258,7 @@ sha1_process_bytes (const void *buffer, size_t len, struct sha1_ctx *ctx)
>  	{
>  	  sha1_process_block (ctx->buffer, 64, ctx);
>  	  left_over -= 64;
> -	  memcpy (ctx->buffer, &ctx->buffer[16], left_over);
> +	  memmove (ctx->buffer, &ctx->buffer[16], left_over);
>  	}
>        ctx->buflen = left_over;
>      }
> -- 
> 2.29.2
> 
> 

Re: [PATCH] Fix potential overlap dest buffer

[Paul Menzel]

Dear Nigel,


Am 17.08.21 um 15:14 schrieb Nigel Croxon:
> To meet requirements of Common Criteria certification vulnerablility

vulnerability

> assessment.

That’s only part of a sentence? Is that supposed to be read as a 
continuation of the commit message summary: … to meet requirements …?

> Static code analysis has been run and found the following
> error.  Overlapping_buffer: The source buffer potentially overlaps

It’d be great, if you denoted, which tool was run. Maybe even add a 
Found-by tag.

> with the destination buffer, which results in undefined
> behavior for "memcpy".
> 
> The change is to use memmove instead of memcpy.
> 
> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
> ---
>   sha1.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sha1.c b/sha1.c
> index 11be7045..89b32f46 100644
> --- a/sha1.c
> +++ b/sha1.c
> @@ -258,7 +258,7 @@ sha1_process_bytes (const void *buffer, size_t len, struct sha1_ctx *ctx)
>   	{
>   	  sha1_process_block (ctx->buffer, 64, ctx);
>   	  left_over -= 64;
> -	  memcpy (ctx->buffer, &ctx->buffer[16], left_over);
> +	  memmove (ctx->buffer, &ctx->buffer[16], left_over);
>   	}
>         ctx->buflen = left_over;
>       }

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

Re: [PATCH] Fix potential overlap dest buffer

[Nigel Croxon]

On 8/18/21 2:34 AM, Paul Menzel wrote:
> Dear Nigel,
> 
> 
> Am 17.08.21 um 15:14 schrieb Nigel Croxon:
>> To meet requirements of Common Criteria certification vulnerablility
> 
> vulnerability
> 
>> assessment.
> 
> That’s only part of a sentence? Is that supposed to be read as a 
> continuation of the commit message summary: … to meet requirements …?
> 

Continuation of the commit message. Thanks for catching the
spelling mistake.

>> Static code analysis has been run and found the following
>> error.  Overlapping_buffer: The source buffer potentially overlaps
> 
> It’d be great, if you denoted, which tool was run. Maybe even add a 
> Found-by tag.
> 

I ran the Coverity scan on the latest build.
https://people.redhat.com/ncroxon/mdadm-4.2-rc2-scan-results.html


>> with the destination buffer, which results in undefined
>> behavior for "memcpy".
>>
>> The change is to use memmove instead of memcpy.
>>
>> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
>> ---
>>   sha1.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sha1.c b/sha1.c
>> index 11be7045..89b32f46 100644
>> --- a/sha1.c
>> +++ b/sha1.c
>> @@ -258,7 +258,7 @@ sha1_process_bytes (const void *buffer, size_t 
>> len, struct sha1_ctx *ctx)
>>       {
>>         sha1_process_block (ctx->buffer, 64, ctx);
>>         left_over -= 64;
>> -      memcpy (ctx->buffer, &ctx->buffer[16], left_over);
>> +      memmove (ctx->buffer, &ctx->buffer[16], left_over);
>>       }
>>         ctx->buflen = left_over;
>>       }
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul
> 

Re: [PATCH] Fix potential overlap dest buffer

[Jes Sorensen]

On 8/17/21 9:14 AM, Nigel Croxon wrote:
> To meet requirements of Common Criteria certification vulnerablility
> assessment. Static code analysis has been run and found the following
> error.  Overlapping_buffer: The source buffer potentially overlaps
> with the destination buffer, which results in undefined
> behavior for "memcpy".
> 
> The change is to use memmove instead of memcpy.
> 
> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
> ---
>  sha1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sha1.c b/sha1.c
> index 11be7045..89b32f46 100644
> --- a/sha1.c
> +++ b/sha1.c
> @@ -258,7 +258,7 @@ sha1_process_bytes (const void *buffer, size_t len, struct sha1_ctx *ctx)
>  	{
>  	  sha1_process_block (ctx->buffer, 64, ctx);
>  	  left_over -= 64;
> -	  memcpy (ctx->buffer, &ctx->buffer[16], left_over);
> +	  memmove (ctx->buffer, &ctx->buffer[16], left_over);
>  	}
>        ctx->buflen = left_over;
>      }
> 

Applied!

Thanks,
Jes