busybox: add patch to fix seg fault in 'ifup -a'

From: "Daniel Mentz" <[email protected]>
To: "buildroot" <[email protected]>
Cc: "ratbert90" <[email protected]>, "DATACOM" <[email protected]>, "Daniel Mentz" <[email protected]>
Sent: S?bado, 10 de novembro de 2018 17:33:19
Subject: [PATCH 1/1] busybox: add patch to fix seg fault in 'ifup -a'

Building busybox for arm64 generates the following warning message:

libbb/get_line_from_file.c: In function ?xmalloc_fgets?:
libbb/get_line_from_file.c:52:38: warning: passing argument 2 of ?bb_get_chunk_from_file? from incompatible pointer type [-Wincompatible-pointer-types]

 return bb_get_chunk_from_file(file, &i);
                                     ^
libbb/get_line_from_file.c:13:17: note: expected ?size_t * {aka long unsigned
int *}? but argument is of type ?int *?
char* FAST_FUNC bb_get_chunk_from_file(FILE *file, size_t *end)
                ^~~~~~~~~~~~~~~~~~~~~~

As it turned out, this is a real bug that leads to stack corruption. The following command crashed on my device due to a NULL pointer being derefenced. That pointer turned out to be a victim of the stack corruption.

/sbin/ifup -a

The affected pointer was liface in ifupdown_main(). The crash occured on the following line:

if (strcmp(liface, currif->iface) == 0) {

liface should have pointed to "eth0" but got corrupted.

Signed-off-by Daniel Mentz <[email protected]>

---
..._fgets-use-size_t-for-bb_get_chunk_f.patch | 27 +++++++++++++++++++

1 file changed, 27 insertions(+)

create mode 100644 package/busybox/0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch

diff --git a/package/busybox/0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch b/package/busybox/0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch new file mode 100644
index 0000000000..62e7cf6c3d
--- /dev/null
+++
b/package/busybox/0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch

(quoted diff) +16 -0

@@ -0,0 +1,27 @@
+From 22a99516206b33b7ae124d426319bab03d5c8309 Mon Sep 17 00:00:00 2001
+From: Denys Vlasenko <[email protected]>
+Date: Sun, 2 Sep 2018 18:48:09 +0200
+Subject: [PATCH] libbb: in xmalloc_fgets(), use size_t for
+ bb_get_chunk_from_file()
+
+Signed-off-by: Denys Vlasenko <[email protected]>
+---
+ libbb/get_line_from_file.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/libbb/get_line_from_file.c b/libbb/get_line_from_file.c
+index 49ef093c2..903ff1fb6 100644
+--- a/libbb/get_line_from_file.c
++++ b/libbb/get_line_from_file.c
+@@ -47,7 +47,7 @@ char* FAST_FUNC bb_get_chunk_from_file(FILE *file, size_t

*end)
+ /* Get line, including trailing \n if any */
+ char* FAST_FUNC xmalloc_fgets(FILE *file)
+ {
+- int i;
++ size_t i;

(quoted diff) +6 -1

+
+ 	return bb_get_chunk_from_file(file, &i);
+ }
+--
+2.17.1
+
--

2.17.1

Quoted 79 lines

From: "Daniel Mentz" <[email protected]>
To: "buildroot" <[email protected]>
Cc: "ratbert90" <[email protected]>, "DATACOM" <[email protected]>, "Daniel Mentz" <[email protected]>
Sent: S?bado, 10 de novembro de 2018 17:33:19
Subject: [PATCH 1/1] busybox: add patch to fix seg fault in 'ifup -a'

Building busybox for arm64 generates the following warning message:

libbb/get_line_from_file.c: In function ?xmalloc_fgets?:
libbb/get_line_from_file.c:52:38: warning: passing argument 2 of ?bb_get_chunk_from_file? from incompatible pointer type [-Wincompatible-pointer-types]

 return bb_get_chunk_from_file(file, &i);
                                     ^
libbb/get_line_from_file.c:13:17: note: expected ?size_t * {aka long unsigned
int *}? but argument is of type ?int *?
char* FAST_FUNC bb_get_chunk_from_file(FILE *file, size_t *end)
                ^~~~~~~~~~~~~~~~~~~~~~

As it turned out, this is a real bug that leads to stack corruption. The following command crashed on my device due to a NULL pointer being derefenced. That pointer turned out to be a victim of the stack corruption.

/sbin/ifup -a

The affected pointer was liface in ifupdown_main(). The crash occured on the following line:

if (strcmp(liface, currif->iface) == 0) {

liface should have pointed to "eth0" but got corrupted.

Signed-off-by Daniel Mentz <[email protected]>

---
..._fgets-use-size_t-for-bb_get_chunk_f.patch | 27 +++++++++++++++++++

1 file changed, 27 insertions(+)

create mode 100644 package/busybox/0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch

diff --git a/package/busybox/0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch b/package/busybox/0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch new file mode 100644
index 0000000000..62e7cf6c3d
--- /dev/null
+++
b/package/busybox/0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch

(quoted diff) +16 -0

@@ -0,0 +1,27 @@
+From 22a99516206b33b7ae124d426319bab03d5c8309 Mon Sep 17 00:00:00 2001
+From: Denys Vlasenko <[email protected]>
+Date: Sun, 2 Sep 2018 18:48:09 +0200
+Subject: [PATCH] libbb: in xmalloc_fgets(), use size_t for
+ bb_get_chunk_from_file()
+
+Signed-off-by: Denys Vlasenko <[email protected]>
+---
+ libbb/get_line_from_file.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/libbb/get_line_from_file.c b/libbb/get_line_from_file.c
+index 49ef093c2..903ff1fb6 100644
+--- a/libbb/get_line_from_file.c
++++ b/libbb/get_line_from_file.c
+@@ -47,7 +47,7 @@ char* FAST_FUNC bb_get_chunk_from_file(FILE *file, size_t

*end)
+ /* Get line, including trailing \n if any */
+ char* FAST_FUNC xmalloc_fgets(FILE *file)
+ {

+-    int i;
++    size_t i;

(quoted diff) +6 -1

+
+     return bb_get_chunk_from_file(file, &i);
+ }
+--
+2.17.1
+
--

Quoted 1 line

2.17.1

Busybox 1.29.3, which is on Buildroot master since commit 77497f5497, aleady has this fix:

Applying 0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch using patch:
patching file libbb/get_line_from_file.c
Reversed (or previously applied) patch detected! Skipping patch.

What Busybox version are you using? Perhaps your patch could be applied on the LTS branches but I think we should just bump it to 1.29.3 on those branches too.

Peter?

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Joel" == Joel Carlson <[email protected]> writes:

Quoted 11 lines

Busybox 1.29.3, which is on Buildroot master since commit 77497f5497, aleady has this fix:

Applying 0004-libbb-in-xmalloc_fgets-use-size_t-for-bb_get_chunk_f.patch using patch:
patching file libbb/get_line_from_file.c
Reversed (or previously applied) patch detected! Skipping patch.

What Busybox version are you using? Perhaps your patch could be applied on the LTS branches but I think we should just bump it to 1.29.3 on those branches too.

Quoted 1 line

Peter?

I'm not Peter (obviously), but I'd recommend bumping the buildroot version on any LTS branches still using busybox 1.29.2. I have a branch off of 2018.08, and I was hitting the same segfault issue until I cherry-picked the commit from master that bumps busybox to 1.29.3. The only change between 1.29.2 and 1.29.3 was the commit to fix this issue.

2018.08.x still had 1.29.2, so I've bumped that to 1.29.3.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Daniel" == Daniel Mentz <[email protected]> writes:

Quoted 1 line · Peter Korsgaard

2018.08.x still had 1.29.2, so I've bumped that to 1.29.3.

You said you bumped busybox on 2018.08.x to 1.29.3? I just ran "git fetch", but I can't see the change in origin/2018.08.x