open-power / petitboot

GNU General Public License v2.0
205 stars 56 forks source link

Setting a directory path for a dtb file does not seem to fail any validation checks #90

Open BradWhittington opened 2 years ago

BradWhittington commented 2 years ago

I'm having an issue with dietpi booting on an odroid-hc4, here's the related issue - https://github.com/MichaIng/DietPi/issues/5336

Petitboot reads the auto detected /boot/boot.scr file, with what resolves to a badly configured file path, but is a valid directory, and writes an empty dtb file to /tmp/ and then fails during boot.

In this case the boot.scr file is setting a partial path that is a valid directory, but not a valid file (i.e. /boot/dtb is the partial path, the full/correct path should be /boot/dtb/amlogic/meson-sm1-odroid-hc4.dtb, but an environment variable is not properly configured in the boot.scr file)

It seems like pb isn't validating that it's opening a directory as a file, and so it doesn't generate a failure / remove the auto boot option, because it succeeds in opening and copying 0 bytes to it's tmp location.

BradWhittington commented 2 years ago

Reading copy_file_secure_dest, and according to https://stackoverflow.com/questions/42876210/c-fopen-opening-directories it would happily proceed to open a directory as a file (c documentation says it would only fail if opening it in r+)

This might resolve it, but I'm copypasting code and don't know c well enough.

diff --git a/lib/file/file.c b/lib/file/file.c
index 6028005..f913b0d 100644
--- a/lib/file/file.c
+++ b/lib/file/file.c
@@ -45,6 +45,14 @@ int copy_file_secure_dest(void *ctx, const char *source_file,
        ssize_t r;
        size_t l1;

+  struct stat statbuf;
+  stat(source_file, &statbuf);
+  if (!S_ISREG(statbuf.st_mode)) {
+               pb_log("%s: unable to stat source file '%s': %m\n",
+                       __func__, source_file);
+                       return -1;
+  }
+
        source_handle = fopen(source_file, "r");
        if (!source_handle) {
                pb_log("%s: unable to open source file '%s': %m\n",