Skip to contentSkip to conversation

fdt: Fix string property comparison overflow

PATCH New Teddy Reed 3 participants 4 messages Jun 4, 2018 Device tree

lore
Share & tools
lore

Teddy Reed

author
#0 lore raw

PATCH 1/1 fdt: Fix string property comparison overflow

FDT property searching can overflow when comparing strings. This will result in undefined behavior.

This check assures that property name lengths do not overrun the string region or the totalsize.

Signed-off-by Teddy Reed <[email protected]>
---
 lib/libfdt/fdt_ro.c      | 5 +++++
 scripts/dtc/libfdt/fdt.c | 2 ++
 2 files changed, 7 insertions(+)
lib/libfdt/fdt_ro.c +5 -0
index b6ca4e0..612f3ac 100644
@@ -42,6 +42,11 @@ const char *fdt_string(const void *fdt, int stroffset)
 static int _fdt_string_eq(const void *fdt, int stroffset,
 			  const char *s, int len)
 {
+	int total_off = fdt_off_dt_strings(fdt) + stroffset;
+	if (total_off + len + 1 < total_off ||
+	    total_off + len + 1 > fdt_totalsize(fdt))
+		return 0;
+
 	const char *p = fdt_string(fdt, stroffset);
 
 	return (strnlen(p, len + 1) == len) && (memcmp(p, s, len) == 0);
scripts/dtc/libfdt/fdt.c +2 -0
index 7855a17..dffd28d 100644
@@ -57,6 +57,8 @@
 
 int fdt_check_header(const void *fdt)
 {
+	if (fdt == NULL)
+		return -FDT_ERR_BADSTRUCTURE;
 	if (fdt_magic(fdt) == FDT_MAGIC) {
 		/* Complete tree */
 		if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/1 fdt: Fix string property comparison overflow

Quoted 5 lines · Teddy Reed

FDT property searching can overflow when comparing strings. This will result in undefined behavior.

This check assures that property name lengths do not overrun the string region or the totalsize.

The lib/libfdt is mostly a sync from upstream dtc [1] so I suspect it's a problem there too and should probably sent and accepted there and it'll then be pulled back in a resync.

Peter

[1] https://git.kernel.org/pub/scm/utils/dtc/dtc.git

Quoted 42 lines
Signed-off-by Teddy Reed <[email protected]>
---
 lib/libfdt/fdt_ro.c      | 5 +++++
 scripts/dtc/libfdt/fdt.c | 2 ++
 2 files changed, 7 insertions(+)
lib/libfdt/fdt_ro.c +5 -0
index b6ca4e0..612f3ac 100644
@@ -42,6 +42,11 @@ const char *fdt_string(const void *fdt, int stroffset)
 static int _fdt_string_eq(const void *fdt, int stroffset,
                          const char *s, int len)
 {
+       int total_off = fdt_off_dt_strings(fdt) + stroffset;
+       if (total_off + len + 1 < total_off ||
+           total_off + len + 1 > fdt_totalsize(fdt))
+               return 0;
+
        const char *p = fdt_string(fdt, stroffset);
 
        return (strnlen(p, len + 1) == len) && (memcmp(p, s, len) == 0);
scripts/dtc/libfdt/fdt.c +2 -1
index 7855a17..dffd28d 100644
@@ -57,6 +57,8 @@
 
 int fdt_check_header(const void *fdt)
 {
+       if (fdt == NULL)
+               return -FDT_ERR_BADSTRUCTURE;
        if (fdt_magic(fdt) == FDT_MAGIC) {
                /* Complete tree */
                if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
--

2.7.4

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

author
#2 lore raw

PATCH 1/1 fdt: Fix string property comparison overflow

Ignore this patch (re: below Peter's comment).

Quoted 10 lines · Peter Robinson
Quoted 5 lines · Teddy Reed

FDT property searching can overflow when comparing strings. This will result in undefined behavior.

This check assures that property name lengths do not overrun the string region or the totalsize.

The lib/libfdt is mostly a sync from upstream dtc [1] so I suspect it's a problem there too and should probably sent and accepted there and it'll then be pulled back in a resync.

Thanks for the heads up Peter, will do!

Quoted 47 lines

Peter

[1] https://git.kernel.org/pub/scm/utils/dtc/dtc.git

Quoted 42 lines
Signed-off-by Teddy Reed <[email protected]>
---
 lib/libfdt/fdt_ro.c      | 5 +++++
 scripts/dtc/libfdt/fdt.c | 2 ++
 2 files changed, 7 insertions(+)
lib/libfdt/fdt_ro.c +5 -0
index b6ca4e0..612f3ac 100644
@@ -42,6 +42,11 @@ const char *fdt_string(const void *fdt, int stroffset)
 static int _fdt_string_eq(const void *fdt, int stroffset,
                          const char *s, int len)
 {
+       int total_off = fdt_off_dt_strings(fdt) + stroffset;
+       if (total_off + len + 1 < total_off ||
+           total_off + len + 1 > fdt_totalsize(fdt))
+               return 0;
+
        const char *p = fdt_string(fdt, stroffset);
 
        return (strnlen(p, len + 1) == len) && (memcmp(p, s, len) == 0);
scripts/dtc/libfdt/fdt.c +2 -1
index 7855a17..dffd28d 100644
@@ -57,6 +57,8 @@
 
 int fdt_check_header(const void *fdt)
 {
+       if (fdt == NULL)
+               return -FDT_ERR_BADSTRUCTURE;
        if (fdt_magic(fdt) == FDT_MAGIC) {
                /* Complete tree */
                if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
--

2.7.4

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Signature · 1 line
Teddy Reed V
#3 lore raw

PATCH 1/1 fdt: Fix string property comparison overflow

In reply to Peter Robinson
Quoted 7 lines · Peter Robinson
Quoted 5 lines · Teddy Reed

FDT property searching can overflow when comparing strings. This will result in undefined behavior.

This check assures that property name lengths do not overrun the string region or the totalsize.

Quoted 7 lines

The lib/libfdt is mostly a sync from upstream dtc [1] so I suspect it's a problem there too and should probably sent and accepted there and it'll then be pulled back in a resync.

Peter

[1] https://git.kernel.org/pub/scm/utils/dtc/dtc.git

Indeed, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180604/ec07a54f/attachment.sig>