[PATCH 1/1] fix cpio CVE-2023-7216

34 views
Skip to first unread message

Roman Fietze

unread,
May 11, 2026, 3:49:40 AM (3 days ago) May 11
to swupdate
From 7a216a1fd02d73473455d72dd8e41abae848c829 Mon Sep 17 00:00:00 2001
From: Roman Fietze <roman.fi...@vaillant-group.com>
Date: Tue, 17 Mar 2026 13:45:02 +0100
Subject: [PATCH] core: check for valid filenames

Fixes CVE-2023-7216.

Signed-off-by: Roman Fietze <roman....@magna.com>
---
 core/stream_interface.c | 10 ++++++++++
 core/util.c             |  7 +++++++
 include/util.h          |  1 +
 test/test_util.c        | 13 ++++++++++++-
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/core/stream_interface.c b/core/stream_interface.c
index a0c73470..06f5cc40 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -121,6 +121,11 @@ static int extract_file_to_tmp(int fd, const char *fname, unsigned long *poffs,
  fdh.filename, fdh.size, max_size);
  goto err;
  }
+ if (!is_filename_valid(fdh.filename)) {
+ ERROR("%s is an invalid filename, aborting", fdh.filename);
+  return -1;
+ }
+
  TRACE("Found file");
  TRACE("\tfilename %s", fdh.filename);
  TRACE("\tsize %u", (unsigned int)fdh.size);
@@ -261,6 +266,11 @@ static int extract_files(int fd, struct swupdate_cfg *software)
  break;
  }
 
+ if (!is_filename_valid(fdh.filename)) {
+ ERROR("%s is an invalid filename, aborting", fdh.filename);
+ return -1;
+ }
+
  TRACE("Found file");
  TRACE("\tfilename %s", fdh.filename);
  TRACE("\tsize %u %s", (unsigned int)fdh.size,
diff --git a/core/util.c b/core/util.c
index 3f62e070..5b397c4f 100644
--- a/core/util.c
+++ b/core/util.c
@@ -1282,3 +1282,10 @@ bool check_same_file(int fd1, int fd2) {
     if(fstat(fd2, &stat2) < 0) return false;
     return (stat1.st_dev == stat2.st_dev) && (stat1.st_ino == stat2.st_ino);
 }
+
+
+bool
+is_filename_valid (const char *file_name)
+{
+    return file_name != NULL && file_name[0] != '/' && strstr(file_name, "../") == NULL;
+}
diff --git a/include/util.h b/include/util.h
index d4874f64..a29495c4 100644
--- a/include/util.h
+++ b/include/util.h
@@ -285,6 +285,7 @@ int read_lines_notify(int fd, char *buf, int buf_size, int *buf_offset,
 long long get_output_size(struct img_type *img, bool strict);
 bool img_check_free_space(struct img_type *img, int fd);
 bool check_same_file(int fd1, int fd2);
+bool is_filename_valid (const char *file_name);
 
 /* location for libubootenv configuration file */
 const char *get_fwenv_config(void);
diff --git a/test/test_util.c b/test/test_util.c
index d6f2c0a0..ef7708c8 100644
--- a/test/test_util.c
+++ b/test/test_util.c
@@ -39,12 +39,23 @@ static void test_util_ustrtoull(void **state)
  assert_string_equal(suffix, ", some fancy things");
 }
 
+static void test_util_is_filename_valid(void **state)
+{
+    (void)state;
+    assert_true(is_filename_valid("a.swu"));
+    assert_true(is_filename_valid( "sub/d.swu"));
+    assert_false(is_filename_valid("/c.swu"));
+    assert_false(is_filename_valid("../b.swu"));
+    assert_false(is_filename_valid("sub/../e.swu"));
+}
+
 int main(void)
 {
  int error_count = 0;
  const struct CMUnitTest util_tests[] = {
      cmocka_unit_test(test_util_ustrtoull),
-     cmocka_unit_test(test_util_size_delimiter_match)
+     cmocka_unit_test(test_util_size_delimiter_match),
+     cmocka_unit_test(test_util_is_filename_valid)
  };
  error_count += cmocka_run_group_tests_name("util", util_tests,
     util_setup, util_teardown);
--
2.51.0


Stefano Babic

unread,
May 11, 2026, 3:53:21 AM (3 days ago) May 11
to Roman Fietze, swupdate
Hi Roman,

On 5/11/26 06:49, 'Roman Fietze' via swupdate wrote:
> From 7a216a1fd02d73473455d72dd8e41abae848c829 Mon Sep 17 00:00:00 2001
> From: Roman Fietze <roman.fi...@vaillant-group.com>
> Date: Tue, 17 Mar 2026 13:45:02 +0100
> Subject: [PATCH] core: check for valid filenames
>
> Fixes CVE-2023-7216.
>
> Signed-off-by: Roman Fietze <roman....@magna.com>
> ---
>  core/stream_interface.c | 10 ++++++++++
>  core/util.c             |  7 +++++++
>  include/util.h          |  1 +
>  test/test_util.c        | 13 ++++++++++++-
>  4 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/core/stream_interface.c b/core/stream_interface.c
> index a0c73470..06f5cc40 100644
> --- a/core/stream_interface.c
> +++ b/core/stream_interface.c
> @@ -121,6 +121,11 @@ static int extract_file_to_tmp(int fd, const char
> *fname, unsigned long *poffs,
> fdh.filename, fdh.size, max_size);
> goto err;
> }
> +if (!is_filename_valid(fdh.filename)) {
> +ERROR("%s is an invalid filename, aborting", fdh.filename);
> + return -1;
> +}
> +
> TRACE("Found file");
> TRACE("\tfilename %s", fdh.filename);
> TRACE("\tsize %u", (unsigned int)fdh.size);
> @@ -261,6 +266,11 @@ static int extract_files(int fd, struct
> swupdate_cfg *software)
> break;
> }
>
> +if (!is_filename_valid(fdh.filename)) {
> +ERROR("%s is an invalid filename, aborting", fdh.filename);
> +return -1;
Apart that this could be integrated, why is this relevant for SWUpdate ?
Which is the use case and which attack can be done with the image to be
installed ?

Best regards,
Stefano Babic

--
_______________________________________________________________________
Nabla Software Engineering GmbH
Hirschstr. 111A | 86156 Augsburg | Tel: +49 821 45592596
Geschäftsführer : Stefano Babic | HRB 40522 Augsburg
E-Mail: sba...@nabladev.com

Roman Fietze

unread,
May 11, 2026, 4:31:19 AM (3 days ago) May 11
to swupdate
Hello Stefano,

> Which is the use case and which attack can be done…

If, the probability is low, an attacker can "provide" an SWU with absolute or relative pathnames, that point to directories in the system, and swupdate is running with the appropriate privileges, which it normally does, the attacker can overwrite critical files in the system when the cpio is unpacked. Cpio provides an new command lines switch --no-absolute-filenames to prevent such attacks. Swupdate simply would have to check the pathnames in the cpio, and reject SWU's with critical paths, because cpio archives used by swupdate normally only contain relative paths starting with a name or "./'.

Roman

Stefano Babic

unread,
May 11, 2026, 5:42:05 AM (2 days ago) May 11
to Roman Fietze, swupdate
Hi Roman,

On 5/11/26 10:31, 'Roman Fietze' via swupdate wrote:
> Hello Stefano,
>
> > Which is the use case and which attack can be done…
>
> If, the probability is low, an attacker can "provide" an SWU with
> absolute or relative pathnames, that point to directories in the system,
> and swupdate is running with the appropriate privileges, which it
> normally does, the attacker can overwrite critical files in the system
> when the cpio is unpacked.

SWUpdate uses the format of cpio, but it hasn't the logic of cpio and it
does not use cpio sources. The filename is used for packing, because a
unique filename is allowed in a single archive, at least using standard
cpio tools. But SWUpdate is extracting the data itself and it loads
generally into the heap (always when streaming is active, that is when
installed-directly is set). The structure for filenames in the SWU is
flat, that is filename is just a name without path, and I am not sure if
SWUpdate will accept filenames with absolute or relative patch, or it
raises an error. If streaming is not active, artifacts are first
extracted in a working dir, and they are never installed on the running
system. It is not the CPIO extractor responsible to install the
artifact, but it is the chosen handler.


> Cpio provides an new command lines switch --
> no-absolute-filenames to prevent such attacks.

But cpio is not running in SWUpdate, just the format is used. For
SWUpdate, each file in CPIO is simply a blob for the handler.

> Swupdate simply would
> have to check the pathnames in the cpio, and reject SWU's with critical
> paths, because cpio archives used by swupdate normally only contain
> relative paths starting with a name or "./'.

cpios / SWUs used by SWUpdate don't have any relative paths because
filename is used. A filename can be "./somnething" or just "something",
but it is irrelevant because the path itself (and the filename) is not used.

Best regards,
Stefano
> Hirschstr. 111A | 86156 Augsburg | Tel: +49 821 45592596 <tel:
> +49%20821%2045592596>
> Geschäftsführer : Stefano Babic | HRB 40522 Augsburg
> E-Mail: sba...@nabladev.com
>
> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion visit https://groups.google.com/d/msgid/
> swupdate/afc321dd-3050-4a97-bcd6-8954c3f193d0n%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/afc321dd-3050-4a97-
> bcd6-8954c3f193d0n%40googlegroups.com?utm_medium=email&utm_source=footer>.

Roman Fietze

unread,
May 12, 2026, 2:54:19 AM (yesterday) May 12
to swupdate
Hello Stefano,

In stream_interface.c the code first uses extract_cpio_header() to get the pathname in the archive. Then a path named output_file is created using TMPDIR and the extracted filename. If the pathname is e.g. "../usr/bin/whatever", this will result in "/tmp/../usr/bin/whatever". After that openfileoutput() is called using output-file, which in turn simply uses open(2).

Of course, all tools creating SWU's do not contain paths with absolute or such relative names, or maybe even paths with subdirs, but the SWU of an attacker could contain such a beast, and fr me it was no problem to do so.

The only thing I overlooked is, that swupdate cleans up all files from the archive when it's done. In my case swupdate removed a faked file named "/tmp/../usr/bin/faked-ls". But this could be anything else.

Best regards,

Roman

Stefano Babic

unread,
May 12, 2026, 3:11:02 AM (yesterday) May 12
to Roman Fietze, swupdate
Hi Roman,

On 5/12/26 08:54, 'Roman Fietze' via swupdate wrote:
> Hello Stefano,
>
> In stream_interface.c the code first uses extract_cpio_header() to get
> the pathname in the archive. Then a path named output_file is created
> using TMPDIR and the extracted filename. If the pathname is e.g. "../
> usr/bin/whatever", this will result in "/tmp/../usr/bin/whatever". After
> that openfileoutput() is called using output-file, which in turn simply
> uses open(2).

Yes, I see this, confirmed, you're right.

>
> Of course, all tools creating SWU's do not contain paths with absolute
> or such relative names, or maybe even paths with subdirs, but the SWU of
> an attacker could contain such a beast, and fr me it was no problem to
> do so.
>
> The only thing I overlooked is, that swupdate cleans up all files from
> the archive when it's done. In my case swupdate removed a faked file
> named "/tmp/../usr/bin/faked-ls". But this could be anything else.
>

Fine with me, I merge the patch.

Thanks,
Stefano
> +49%20821%2045592596> <tel:
> > +49%20821%2045592596>
> > Geschäftsführer : Stefano Babic | HRB 40522 Augsburg
> > E-Mail: sba...@nabladev.com
> >
> > --
> > You received this message because you are subscribed to the Google
> > Groups "swupdate" group.
> > To unsubscribe from this group and stop receiving emails from it,
> send
> > an email to swupdate+u...@googlegroups.com
> > <mailto:swupdate+u...@googlegroups.com>.
> > To view this discussion visit https://groups.google.com/d/msgid/
> <https://groups.google.com/d/msgid/>
> > swupdate/afc321dd-3050-4a97-bcd6-8954c3f193d0n%40googlegroups.com
> <http://40googlegroups.com>
> > <https://groups.google.com/d/msgid/swupdate/afc321dd-3050-4a97-
> <https://groups.google.com/d/msgid/swupdate/afc321dd-3050-4a97->
> > bcd6-8954c3f193d0n%40googlegroups.com?
> utm_medium=email&utm_source=footer <http://40googlegroups.com?
> utm_medium=email&utm_source=footer>>.
>
> --
> _______________________________________________________________________
> Nabla Software Engineering GmbH
> Hirschstr. 111A | 86156 Augsburg | Tel: +49 821 45592596 <tel:
> +49%20821%2045592596>
> Geschäftsführer : Stefano Babic | HRB 40522 Augsburg
> E-Mail: sba...@nabladev.com
>
> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion visit https://groups.google.com/d/msgid/
> swupdate/aa0a8a48-178c-488d-b750-c4c984f8ef8cn%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/aa0a8a48-178c-488d-b750-
> c4c984f8ef8cn%40googlegroups.com?utm_medium=email&utm_source=footer>.
Reply all
Reply to author
Forward
0 new messages