diff mbox

[for,2.10,31/35] syscall: replace strcpy() by g_strlcpy()

Message ID 20170724182751.18261-32-f4bug@amsat.org
State New
Headers show

Commit Message

Philippe Mathieu-Daudé July 24, 2017, 6:27 p.m. UTC
linux-user/syscall.c:9860:17: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
                strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
                ^~~~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 linux-user/syscall.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Laurent Vivier July 24, 2017, 7:28 p.m. UTC | #1
Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit :
> linux-user/syscall.c:9860:17: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
>                 strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
>                 ^~~~~~
> 
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  linux-user/syscall.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 963b9c8f4b..847f729834 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9853,7 +9853,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              if (!is_error(ret)) {
>                  /* Overwrite the native machine name with whatever is being
>                     emulated. */
> -                strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
> +                g_strlcpy(buf->machine, cpu_to_uname_machine(cpu_env),
> +                          sizeof(buf->machine));
>                  /* Allow the user to override the reported release.  */
>                  if (qemu_uname_release && *qemu_uname_release) {
>                      g_strlcpy(buf->release, qemu_uname_release,
> 

We should not have a problem here as cpu_to_uname_machine() is "const
char *" and the string is defined inside QEMU (so it should fit into
machine[]).

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Philippe Mathieu-Daudé May 29, 2018, 2:19 p.m. UTC | #2
Hi Laurent,

On 07/24/2017 04:28 PM, Laurent Vivier wrote:
> Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit :
>> linux-user/syscall.c:9860:17: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
>>                 strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
>>                 ^~~~~~
>>
>> Reported-by: Clang Static Analyzer
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  linux-user/syscall.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 963b9c8f4b..847f729834 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -9853,7 +9853,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>              if (!is_error(ret)) {
>>                  /* Overwrite the native machine name with whatever is being
>>                     emulated. */
>> -                strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
>> +                g_strlcpy(buf->machine, cpu_to_uname_machine(cpu_env),
>> +                          sizeof(buf->machine));
>>                  /* Allow the user to override the reported release.  */
>>                  if (qemu_uname_release && *qemu_uname_release) {
>>                      g_strlcpy(buf->release, qemu_uname_release,
>>
> 
> We should not have a problem here as cpu_to_uname_machine() is "const
> char *" and the string is defined inside QEMU (so it should fit into
> machine[]).
> 
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Do you mind queuing this patch in your linux-user tree?

Thanks,

Phil.
Laurent Vivier May 29, 2018, 3:22 p.m. UTC | #3
Le 29/05/2018 à 16:19, Philippe Mathieu-Daudé a écrit :
> Hi Laurent,
> 
> On 07/24/2017 04:28 PM, Laurent Vivier wrote:
>> Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit :
>>> linux-user/syscall.c:9860:17: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
>>>                 strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
>>>                 ^~~~~~
>>>
>>> Reported-by: Clang Static Analyzer
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  linux-user/syscall.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 963b9c8f4b..847f729834 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -9853,7 +9853,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>>              if (!is_error(ret)) {
>>>                  /* Overwrite the native machine name with whatever is being
>>>                     emulated. */
>>> -                strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
>>> +                g_strlcpy(buf->machine, cpu_to_uname_machine(cpu_env),
>>> +                          sizeof(buf->machine));
>>>                  /* Allow the user to override the reported release.  */
>>>                  if (qemu_uname_release && *qemu_uname_release) {
>>>                      g_strlcpy(buf->release, qemu_uname_release,
>>>
>>
>> We should not have a problem here as cpu_to_uname_machine() is "const
>> char *" and the string is defined inside QEMU (so it should fit into
>> machine[]).
>>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 
> Do you mind queuing this patch in your linux-user tree?

Applied, thanks

Laurent
diff mbox

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 963b9c8f4b..847f729834 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9853,7 +9853,8 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (!is_error(ret)) {
                 /* Overwrite the native machine name with whatever is being
                    emulated. */
-                strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
+                g_strlcpy(buf->machine, cpu_to_uname_machine(cpu_env),
+                          sizeof(buf->machine));
                 /* Allow the user to override the reported release.  */
                 if (qemu_uname_release && *qemu_uname_release) {
                     g_strlcpy(buf->release, qemu_uname_release,