Re: [LEDE-DEV] [PATCH 1/2] firmware-utils: mkdapimg2: Fix potential buffer overflow

2018-04-24 Thread Hauke Mehrtens 
On 04/23/2018 01:22 AM, Rosen Penev wrote:
> If GCC is built with stack smashing protection enabled (SSP), it errors when 
> compiling lzma-loader.
> 
> Switching to strncpy seems to be an easy way to fix this. It's probably 
> better to use snprintf or strlcpy but the latter is not available for glibc 
> systems.

I also saw this problem, because my version number got too long and it
just failed because the first byte of the signature contained now the
terminating NULL byte of the version.

Does anybody know if the version number, signature and so on has to be
NULL terminated or not?

I created this patch some days ago to not run into this problem any more:
https://git.openwrt.org/?p=openwrt/staging/hauke.git;a=commitdiff;h=b69a5d5ee4c7611a45693eec481f259520eb6422


> Output of crash below:
> 
> *** buffer overflow detected ***: 
> /home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2 terminated
> === Backtrace: =
> /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7f6318ea77e5]
> /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7f6318f4915c]
> /lib/x86_64-linux-gnu/libc.so.6(+0x117160)[0x7f6318f47160]
> /home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2[0x400ab7]
> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6318e50830]
> /home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2[0x400e69]
> === Memory map: 
> 0040-00402000 r-xp  00:00 223275 
> /home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg200601000-00602000 
> r--p 1000 00:00 223275 
> /home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg200602000-00603000 
> rw-p 2000 00:00 223275 
> /home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2018c2000-018e3000 
> rw-p  00:00 0  [heap]
> 7f6318c1-7f6318c26000 r-xp  00:00 122040 
> /lib/x86_64-linux-gnu/libgcc_s.so.1
> 7f6318c26000-7f6318e25000 ---p 00016000 00:00 122040 
> /lib/x86_64-linux-gnu/libgcc_s.so.1
> 7f6318e25000-7f6318e26000 rw-p 00015000 00:00 122040 
> /lib/x86_64-linux-gnu/libgcc_s.so.1
> 7f6318e3-7f6318ff r-xp  00:00 123971 
> /lib/x86_64-linux-gnu/libc-2.23.so
> 7f6318ff-7f6318ff9000 ---p 001c 00:00 123971 
> /lib/x86_64-linux-gnu/libc-2.23.so
> 7f6318ff9000-7f63191f ---p 001c9000 00:00 123971 
> /lib/x86_64-linux-gnu/libc-2.23.so
> 7f63191f-7f63191f4000 r--p 001c 00:00 123971 
> /lib/x86_64-linux-gnu/libc-2.23.so
> 7f63191f4000-7f63191f6000 rw-p 001c4000 00:00 123971 
> /lib/x86_64-linux-gnu/libc-2.23.so
> 7f63191f6000-7f63191fa000 rw-p  00:00 0
> 7f631920-7f6319226000 r-xp  00:00 123969 
> /lib/x86_64-linux-gnu/ld-2.23.so
> 7f6319425000-7f6319426000 r--p 00025000 00:00 123969 
> /lib/x86_64-linux-gnu/ld-2.23.so
> 7f6319426000-7f6319427000 rw-p 00026000 00:00 123969 
> /lib/x86_64-linux-gnu/ld-2.23.so
> 7f6319427000-7f6319428000 rw-p  00:00 0
> 7f631946-7f6319461000 rw-p  00:00 0
> 7f631947-7f6319471000 rw-p  00:00 0
> 7f631948-7f6319481000 rw-p  00:00 0
> 7f631949-7f6319491000 rw-p  00:00 0
> 7fffca52e000-7fffcad2e000 rw-p  00:00 0  [stack]
> 7fffcb121000-7fffcb122000 r-xp  00:00 0  [vdso]
> make -r world: build failed. Please re-run make with -j1 V=s to see what's 
> going on
> 
> Signed-off-by: Rosen Penev 
> ---
>  tools/firmware-utils/src/mkdapimg2.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/firmware-utils/src/mkdapimg2.c 
> b/tools/firmware-utils/src/mkdapimg2.c
> index aef003c..59c9f00 100644
> --- a/tools/firmware-utils/src/mkdapimg2.c
> +++ b/tools/firmware-utils/src/mkdapimg2.c
> @@ -119,7 +119,7 @@ main(int ac, char *av[])
>   progname, MAX_SIGN_LEN);
>   exit(1);
>   }
> - strcpy(signature, optarg);
> + strncpy(signature, optarg, MAX_SIGN_LEN);
>   break;
>   case 'v':
>   if (strlen(optarg) > MAX_FW_VER_LEN + 1) {
> @@ -127,7 +127,7 @@ main(int ac, char *av[])
>   progname, MAX_FW_VER_LEN);
>   exit(1);
>   }
> - strcpy(version, optarg);
> + strncpy(version, optarg, MAX_FW_VER_LEN);
>   break;
>   case 'r':
>   if (strlen(optarg) > MAX_REG_LEN + 1) {
> @@ -135,7 +135,7 @@ main(int ac, char *av[])
>   progname, MAX_REG_LEN);
>   exit(1);
>   }
> - strcpy(region, optarg);
> +

Re: [LEDE-DEV] [PATCH 1/2] firmware-utils: mkdapimg2: Fix potential buffer overflow

2018-04-23 Thread Arjen de Korte 

Citeren Rosen Penev :

If GCC is built with stack smashing protection enabled (SSP), it  
errors when compiling lzma-loader.


Switching to strncpy seems to be an easy way to fix this. It's  
probably better to use snprintf or strlcpy but the latter is not  
available for glibc systems.


This seems to be the wrong solution to a problem. For each of the  
strings involved, storage is allocated on the stack. There is a length  
check before the contents of the arguments provided, is copied.  
However, the values checked against are incorrect. The required  
storage length for a string will be strlen(string) + 1 (to allow for  
the terminating NUL byte).


So instead of

117 if (strlen(optarg) > MAX_SIGN_LEN + 1) {
125 if (strlen(optarg) > MAX_FW_VER_LEN + 1) {
133 if (strlen(optarg) > MAX_REG_LEN + 1) {

the correct check would have been

117 if (strlen(optarg) > MAX_SIGN_LEN - 1) {
125 if (strlen(optarg) > MAX_FW_VER_LEN - 1) {
133 if (strlen(optarg) > MAX_REG_LEN - 1) {

Good catch though.


Output of crash below:

*** buffer overflow detected ***:  
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2  
terminated

=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7f6318ea77e5]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7f6318f4915c]
/lib/x86_64-linux-gnu/libc.so.6(+0x117160)[0x7f6318f47160]
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2[0x400ab7]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6318e50830]
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2[0x400e69]
=== Memory map: 
0040-00402000 r-xp  00:00 223275  
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg200601000-00602000 r--p 1000 00:00 223275 /home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg200602000-00603000 rw-p 2000 00:00 223275 /home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2018c2000-018e3000 rw-p  00:00 0   
[heap]
7f6318c1-7f6318c26000 r-xp  00:00 122040  
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f6318c26000-7f6318e25000 ---p 00016000 00:00 122040  
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f6318e25000-7f6318e26000 rw-p 00015000 00:00 122040  
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f6318e3-7f6318ff r-xp  00:00 123971  
/lib/x86_64-linux-gnu/libc-2.23.so
7f6318ff-7f6318ff9000 ---p 001c 00:00 123971  
/lib/x86_64-linux-gnu/libc-2.23.so
7f6318ff9000-7f63191f ---p 001c9000 00:00 123971  
/lib/x86_64-linux-gnu/libc-2.23.so
7f63191f-7f63191f4000 r--p 001c 00:00 123971  
/lib/x86_64-linux-gnu/libc-2.23.so
7f63191f4000-7f63191f6000 rw-p 001c4000 00:00 123971  
/lib/x86_64-linux-gnu/libc-2.23.so

7f63191f6000-7f63191fa000 rw-p  00:00 0
7f631920-7f6319226000 r-xp  00:00 123969  
/lib/x86_64-linux-gnu/ld-2.23.so
7f6319425000-7f6319426000 r--p 00025000 00:00 123969  
/lib/x86_64-linux-gnu/ld-2.23.so
7f6319426000-7f6319427000 rw-p 00026000 00:00 123969  
/lib/x86_64-linux-gnu/ld-2.23.so

7f6319427000-7f6319428000 rw-p  00:00 0
7f631946-7f6319461000 rw-p  00:00 0
7f631947-7f6319471000 rw-p  00:00 0
7f631948-7f6319481000 rw-p  00:00 0
7f631949-7f6319491000 rw-p  00:00 0
7fffca52e000-7fffcad2e000 rw-p  00:00 0  [stack]
7fffcb121000-7fffcb122000 r-xp  00:00 0  [vdso]
make -r world: build failed. Please re-run make with -j1 V=s to see  
what's going on


Signed-off-by: Rosen Penev 
---
 tools/firmware-utils/src/mkdapimg2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/firmware-utils/src/mkdapimg2.c  
b/tools/firmware-utils/src/mkdapimg2.c

index aef003c..59c9f00 100644
--- a/tools/firmware-utils/src/mkdapimg2.c
+++ b/tools/firmware-utils/src/mkdapimg2.c
@@ -119,7 +119,7 @@ main(int ac, char *av[])
progname, MAX_SIGN_LEN);
exit(1);
}
-   strcpy(signature, optarg);
+   strncpy(signature, optarg, MAX_SIGN_LEN);
break;
case 'v':
if (strlen(optarg) > MAX_FW_VER_LEN + 1) {
@@ -127,7 +127,7 @@ main(int ac, char *av[])
progname, MAX_FW_VER_LEN);
exit(1);
}
-   strcpy(version, optarg);
+   strncpy(version, optarg, MAX_FW_VER_LEN);
break;
case 'r':
if (strlen(optarg) > MAX_REG_LEN + 1) {
@@ -135,7 +135,7 @@ main(int ac, char *av[])

[LEDE-DEV] [PATCH 1/2] firmware-utils: mkdapimg2: Fix potential buffer overflow

2018-04-22 Thread Rosen Penev 
If GCC is built with stack smashing protection enabled (SSP), it errors when 
compiling lzma-loader.

Switching to strncpy seems to be an easy way to fix this. It's probably better 
to use snprintf or strlcpy but the latter is not available for glibc systems.

Output of crash below:

*** buffer overflow detected ***: 
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2 terminated
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7f6318ea77e5]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7f6318f4915c]
/lib/x86_64-linux-gnu/libc.so.6(+0x117160)[0x7f6318f47160]
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2[0x400ab7]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6318e50830]
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2[0x400e69]
=== Memory map: 
0040-00402000 r-xp  00:00 223275 
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg200601000-00602000 
r--p 1000 00:00 223275 
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg200602000-00603000 
rw-p 2000 00:00 223275 
/home/mangix/devstuff/openwrt/staging_dir/host/bin/mkdapimg2018c2000-018e3000 
rw-p  00:00 0  [heap]
7f6318c1-7f6318c26000 r-xp  00:00 122040 
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f6318c26000-7f6318e25000 ---p 00016000 00:00 122040 
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f6318e25000-7f6318e26000 rw-p 00015000 00:00 122040 
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f6318e3-7f6318ff r-xp  00:00 123971 
/lib/x86_64-linux-gnu/libc-2.23.so
7f6318ff-7f6318ff9000 ---p 001c 00:00 123971 
/lib/x86_64-linux-gnu/libc-2.23.so
7f6318ff9000-7f63191f ---p 001c9000 00:00 123971 
/lib/x86_64-linux-gnu/libc-2.23.so
7f63191f-7f63191f4000 r--p 001c 00:00 123971 
/lib/x86_64-linux-gnu/libc-2.23.so
7f63191f4000-7f63191f6000 rw-p 001c4000 00:00 123971 
/lib/x86_64-linux-gnu/libc-2.23.so
7f63191f6000-7f63191fa000 rw-p  00:00 0
7f631920-7f6319226000 r-xp  00:00 123969 
/lib/x86_64-linux-gnu/ld-2.23.so
7f6319425000-7f6319426000 r--p 00025000 00:00 123969 
/lib/x86_64-linux-gnu/ld-2.23.so
7f6319426000-7f6319427000 rw-p 00026000 00:00 123969 
/lib/x86_64-linux-gnu/ld-2.23.so
7f6319427000-7f6319428000 rw-p  00:00 0
7f631946-7f6319461000 rw-p  00:00 0
7f631947-7f6319471000 rw-p  00:00 0
7f631948-7f6319481000 rw-p  00:00 0
7f631949-7f6319491000 rw-p  00:00 0
7fffca52e000-7fffcad2e000 rw-p  00:00 0  [stack]
7fffcb121000-7fffcb122000 r-xp  00:00 0  [vdso]
make -r world: build failed. Please re-run make with -j1 V=s to see what's 
going on

Signed-off-by: Rosen Penev 
---
 tools/firmware-utils/src/mkdapimg2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/firmware-utils/src/mkdapimg2.c 
b/tools/firmware-utils/src/mkdapimg2.c
index aef003c..59c9f00 100644
--- a/tools/firmware-utils/src/mkdapimg2.c
+++ b/tools/firmware-utils/src/mkdapimg2.c
@@ -119,7 +119,7 @@ main(int ac, char *av[])
progname, MAX_SIGN_LEN);
exit(1);
}
-   strcpy(signature, optarg);
+   strncpy(signature, optarg, MAX_SIGN_LEN);
break;
case 'v':
if (strlen(optarg) > MAX_FW_VER_LEN + 1) {
@@ -127,7 +127,7 @@ main(int ac, char *av[])
progname, MAX_FW_VER_LEN);
exit(1);
}
-   strcpy(version, optarg);
+   strncpy(version, optarg, MAX_FW_VER_LEN);
break;
case 'r':
if (strlen(optarg) > MAX_REG_LEN + 1) {
@@ -135,7 +135,7 @@ main(int ac, char *av[])
progname, MAX_REG_LEN);
exit(1);
}
-   strcpy(region, optarg);
+   strncpy(region, optarg, MAX_REG_LEN);
break;
case 'k':
kernel = strtoul(optarg, , 0);
-- 
2.7.4


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev