mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH master 1/3] treewide: fix -Wformat-security warnings for run_command()
@ 2026-03-02 13:52 Ahmad Fatoum
  2026-03-02 13:52 ` [PATCH master 2/3] jwt: fix buffer overflow and double-free in jwt_part_parse Ahmad Fatoum
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2026-03-02 13:52 UTC (permalink / raw)
  To: barebox; +Cc: Claude Opus 4.6, Ahmad Fatoum

run_command() is declared __printf(1, 2), so passing a non-literal
format string triggers -Wformat-security with clang. Use "%s" as the
format string at all call sites that forward a dynamic string.

Reported-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
Targetting master with the assumption that what's in next now will go
into master.
---
 commands/exec.c          | 2 +-
 commands/time.c          | 2 +-
 commands/watch.c         | 2 +-
 common/boot.c            | 2 +-
 common/fastboot.c        | 2 +-
 common/menu.c            | 2 +-
 common/menutree.c        | 2 +-
 common/parser.c          | 2 +-
 common/ratp/ratp.c       | 2 +-
 common/startup.c         | 2 +-
 common/structio.c        | 4 ++--
 fs/fs.c                  | 2 +-
 net/ifup.c               | 2 +-
 security/password.c      | 2 +-
 test/self/test_command.c | 2 +-
 15 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/commands/exec.c b/commands/exec.c
index 0b063181b247..962ba8a99eff 100644
--- a/commands/exec.c
+++ b/commands/exec.c
@@ -26,7 +26,7 @@ static int do_exec(int argc, char *argv[])
 		if (!script)
 			return 1;
 
-		if (run_command(script) == -1)
+		if (run_command("%s", script) == -1)
 			goto out;
 		free(script);
 	}
diff --git a/commands/time.c b/commands/time.c
index a3f270407122..350dc08ab617 100644
--- a/commands/time.c
+++ b/commands/time.c
@@ -34,7 +34,7 @@ static int do_time(int argc, char *argv[])
 
 	start = get_time_ns();
 
-	run_command(buf);
+	run_command("%s", buf);
 
 	end = get_time_ns();
 
diff --git a/commands/watch.c b/commands/watch.c
index 64b59abb107d..82a1934c074f 100644
--- a/commands/watch.c
+++ b/commands/watch.c
@@ -68,7 +68,7 @@ static int do_watch(int argc , char *argv[])
 			printf("%s\n\n", header);
 		}
 
-		run_command(cmd);
+		run_command("%s", cmd);
 
 		start = get_time_ns();
 		while (!is_timeout(start, period_ns)) {
diff --git a/common/boot.c b/common/boot.c
index 3c7f541163a1..0fa2022be1ac 100644
--- a/common/boot.c
+++ b/common/boot.c
@@ -107,7 +107,7 @@ static int bootscript_boot(struct bootentry *entry, int verbose, int dryrun)
 
 	bootm_nattempts = bootm_command_attempts();
 
-	ret = run_command(bs->entry.path);
+	ret = run_command("%s", bs->entry.path);
 	if (ret) {
 		pr_err("Running script '%s' failed: %s\n", bs->entry.path, strerror(-ret));
 		goto out;
diff --git a/common/fastboot.c b/common/fastboot.c
index 84bda241aea1..106072c7616e 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -945,7 +945,7 @@ static void cb_oem_exec(struct fastboot *fb, const char *cmd)
 		return;
 	}
 
-	ret = run_command(cmd);
+	ret = run_command("%s", cmd);
 	if (ret < 0)
 		fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, "%pe", ERR_PTR(ret));
 	else if (ret > 0)
diff --git a/common/menu.c b/common/menu.c
index c985f2987751..895671507796 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -466,7 +466,7 @@ static void menu_action_command(struct menu *m, struct menu_entry *me)
 	if (!s)
 		s = e->command;
 
-	ret = run_command(s);
+	ret = run_command("%s", s);
 
 	if (ret < 0)
 		udelay(1000000);
diff --git a/common/menutree.c b/common/menutree.c
index 196c2f49fa58..6370ad1f56aa 100644
--- a/common/menutree.c
+++ b/common/menutree.c
@@ -29,7 +29,7 @@ static void menutree_action(struct menu *m, struct menu_entry *me)
 {
 	struct menutree *mt = container_of(me, struct menutree, me);
 
-	run_command(mt->action);
+	run_command("%s", mt->action);
 }
 
 static void setenv_bool(const char *var, bool val)
diff --git a/common/parser.c b/common/parser.c
index 50e0b93e30ee..3233d06fe8a4 100644
--- a/common/parser.c
+++ b/common/parser.c
@@ -305,7 +305,7 @@ int run_shell(void)
 		if (len == -1) {
 			puts ("<INTERRUPT>\n");
 		} else {
-			const int rc = run_command(lastcommand);
+			const int rc = run_command("%s", lastcommand);
 			if (rc < 0) {
 				/* invalid command or not repeatable, forget it */
 				lastcommand[0] = 0;
diff --git a/common/ratp/ratp.c b/common/ratp/ratp.c
index f2735fa88531..bbed34d65021 100644
--- a/common/ratp/ratp.c
+++ b/common/ratp/ratp.c
@@ -329,7 +329,7 @@ static void ratp_command_run(struct work_struct *w)
 
 	pr_debug("running command: %s\n", rw->command);
 
-	ret = run_command(rw->command);
+	ret = run_command("%s", rw->command);
 
 	free(rw->command);
 	free(rw);
diff --git a/common/startup.c b/common/startup.c
index dd643182043f..2e2b5f820fe9 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -359,7 +359,7 @@ static int run_init(void)
 		path = &scr[strlen("source ")];
 		if (stat(path, &s) == 0) {
 			pr_info("Invoking '%s'...\n", path);
-			run_command(scr);
+			run_command("%s", scr);
 		}
 		free(scr);
 	}
diff --git a/common/structio.c b/common/structio.c
index 776dc1e902ab..7116617bb6ee 100644
--- a/common/structio.c
+++ b/common/structio.c
@@ -18,12 +18,12 @@ int structio_run_command(struct bobject **bret, const char *cmd)
 	int ret;
 
 	if (!bret)
-		return run_command(cmd);
+		return run_command("%s", cmd);
 
 	active_capture = bobj = bobject_alloc("capture");
 	bobj->local = true;
 
-	ret = run_command(cmd);
+	ret = run_command("%s", cmd);
 
 	active_capture = NULL;
 
diff --git a/fs/fs.c b/fs/fs.c
index 43840c3a7ace..6a73a5baa26e 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -3507,7 +3507,7 @@ static int automount_mount(struct dentry *dentry)
 
 		setenv("automount_path", am->path);
 		export("automount_path");
-		ret = run_command(am->cmd);
+		ret = run_command("%s", am->cmd);
 		unsetenv("automount_path");
 
 		if (ret) {
diff --git a/net/ifup.c b/net/ifup.c
index bd821535e8b3..9e87cfc58f7e 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -31,7 +31,7 @@ static int eth_discover(char *file)
 		goto out;
 	}
 
-	ret = run_command(file);
+	ret = run_command("%s", file);
 	if (ret) {
 		pr_err("Running '%s' failed with %d\n", file, ret);
 		goto out;
diff --git a/security/password.c b/security/password.c
index 55b2d1093ab9..8067008d5126 100644
--- a/security/password.c
+++ b/security/password.c
@@ -417,7 +417,7 @@ void login(void)
 
 		ret = password(passwd, PASSWD_MAX_LENGTH, LOGIN_MODE, login_timeout);
 		if (ret < 0)
-			run_command(login_fail_command);
+			run_command("%s", login_fail_command);
 
 		if (ret < 0)
 			continue;
diff --git a/test/self/test_command.c b/test/self/test_command.c
index 358855d0f68a..b545e5c09eb0 100644
--- a/test/self/test_command.c
+++ b/test/self/test_command.c
@@ -25,7 +25,7 @@ static void __assert_eq(const char *expr, bool result, const char *func, int lin
 
 	total_tests++;
 
-	ret = run_command(expr);
+	ret = run_command("%s", expr);
 	if ((result && ret != 0) || (!result && ret != 1)) {
 		failed_tests++;
 		printf("%s:%d: %s: assertion failure, ret=%d\n", func, line, expr, ret);
-- 
2.47.3




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH master 2/3] jwt: fix buffer overflow and double-free in jwt_part_parse
  2026-03-02 13:52 [PATCH master 1/3] treewide: fix -Wformat-security warnings for run_command() Ahmad Fatoum
@ 2026-03-02 13:52 ` Ahmad Fatoum
  2026-03-02 13:52 ` [PATCH master 3/3] of: fdt: fix heap-buffer-overflow in fdt_machine_is_compatible Ahmad Fatoum
  2026-03-04  7:34 ` [PATCH master 1/3] treewide: fix -Wformat-security warnings for run_command() Sascha Hauer
  2 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2026-03-02 13:52 UTC (permalink / raw)
  To: barebox; +Cc: Claude Opus 4.6, Ahmad Fatoum

jwt_part_parse() allocates a buffer with xmalloc(len) and then writes
a NUL terminator at decoded_len, but when len is 0 (empty JWT parts
like "..sig"), this writes past the allocation.

Additionally, when jsmn_parse_alloc() fails, the function frees
part->content but doesn't NULL the pointer. The caller then calls
jwt_free() → jwt_part_free() which frees part->content again.

Fix both: allocate len + 1 to accommodate the NUL terminator, and
NULL out part->content after freeing it on the error path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 security/jwt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/jwt.c b/security/jwt.c
index e4be17dcfac0..e828ccfd8cfe 100644
--- a/security/jwt.c
+++ b/security/jwt.c
@@ -55,12 +55,13 @@ static int jwt_part_parse(struct jwt_part *part, const char *content, size_t len
 {
 	size_t decoded_len;
 
-	part->content = xmalloc(len);
+	part->content = xmalloc(len + 1);
 	decoded_len = decode_base64url(part->content, len, content);
 	part->content[decoded_len] = '\0';
 	part->tokens = jsmn_parse_alloc(part->content, decoded_len, &part->token_count);
 	if (!part->tokens) {
 		free(part->content);
+		part->content = NULL;
 		return -EILSEQ;
 	}
 
-- 
2.47.3




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH master 3/3] of: fdt: fix heap-buffer-overflow in fdt_machine_is_compatible
  2026-03-02 13:52 [PATCH master 1/3] treewide: fix -Wformat-security warnings for run_command() Ahmad Fatoum
  2026-03-02 13:52 ` [PATCH master 2/3] jwt: fix buffer overflow and double-free in jwt_part_parse Ahmad Fatoum
@ 2026-03-02 13:52 ` Ahmad Fatoum
  2026-03-04  7:34 ` [PATCH master 1/3] treewide: fix -Wformat-security warnings for run_command() Sascha Hauer
  2 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2026-03-02 13:52 UTC (permalink / raw)
  To: barebox; +Cc: Claude Opus 4.6, Ahmad Fatoum

fdt_machine_is_compatible() reads the FDT property header fields
(len, nameoff) without first checking that the full struct fdt_property
fits within the FDT buffer. While the tag field (4 bytes) is validated
by dt_ptr_ok(fdt, tagp), the subsequent access to fdt_prop->len and
fdt_prop->nameoff (at offsets +4 and +8) can read past the buffer when
the FDT is truncated between the tag and the property header fields.

Add a dt_ptr_ok(fdt, fdt_prop) check before accessing the property
fields, matching the existing pattern in __of_unflatten_dtb().

Found by fuzz-fdt-compatible with AddressSanitizer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 drivers/of/fdt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 658718d4e238..13fb4b5c099e 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -825,6 +825,9 @@ int fdt_machine_is_compatible(const struct fdt_header *fdt, size_t fdt_size, con
 
 		case FDT_PROP:
 			fdt_prop = (const void *)fdt + dt_struct;
+			if (!dt_ptr_ok(fdt, fdt_prop))
+				return 0;
+
 			len = fdt32_to_cpu(fdt_prop->len);
 
 			name = dt_string(&f, dt_strings, fdt32_to_cpu(fdt_prop->nameoff));
-- 
2.47.3




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH master 1/3] treewide: fix -Wformat-security warnings for run_command()
  2026-03-02 13:52 [PATCH master 1/3] treewide: fix -Wformat-security warnings for run_command() Ahmad Fatoum
  2026-03-02 13:52 ` [PATCH master 2/3] jwt: fix buffer overflow and double-free in jwt_part_parse Ahmad Fatoum
  2026-03-02 13:52 ` [PATCH master 3/3] of: fdt: fix heap-buffer-overflow in fdt_machine_is_compatible Ahmad Fatoum
@ 2026-03-04  7:34 ` Sascha Hauer
  2 siblings, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2026-03-04  7:34 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum; +Cc: Claude Opus 4.6


On Mon, 02 Mar 2026 14:52:32 +0100, Ahmad Fatoum wrote:
> run_command() is declared __printf(1, 2), so passing a non-literal
> format string triggers -Wformat-security with clang. Use "%s" as the
> format string at all call sites that forward a dynamic string.
> 
> 

Applied, thanks!

[1/3] treewide: fix -Wformat-security warnings for run_command()
      https://git.pengutronix.de/cgit/barebox/commit/?id=f6e2a02f918f (link may not be stable)
[2/3] jwt: fix buffer overflow and double-free in jwt_part_parse
      https://git.pengutronix.de/cgit/barebox/commit/?id=ca9205326237 (link may not be stable)
[3/3] of: fdt: fix heap-buffer-overflow in fdt_machine_is_compatible
      https://git.pengutronix.de/cgit/barebox/commit/?id=ef2e9ab5611c (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-04  7:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-02 13:52 [PATCH master 1/3] treewide: fix -Wformat-security warnings for run_command() Ahmad Fatoum
2026-03-02 13:52 ` [PATCH master 2/3] jwt: fix buffer overflow and double-free in jwt_part_parse Ahmad Fatoum
2026-03-02 13:52 ` [PATCH master 3/3] of: fdt: fix heap-buffer-overflow in fdt_machine_is_compatible Ahmad Fatoum
2026-03-04  7:34 ` [PATCH master 1/3] treewide: fix -Wformat-security warnings for run_command() Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox