Message ID | 20170724182751.18261-32-f4bug@amsat.org |
---|---|
State | New |
Headers | show |
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>
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.
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 --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,
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(-)