commit ec9409e1b220ac6f77665add17c8f6f1c9642156
author Vincent Palatin <vpalatin@chromium.org> Thu Apr 08 13:49:08 2021 +0200
committer Commit Bot <commit-bot@chromium.org> Wed Apr 21 17:13:09 2021 +0000
tree 4cbb7b29211f6d74ff039e825186091ad0518c23
parent c5bdc537bce51c222c05d215b69bf4eb5948c665

modemfwd: fix check for invalid firmware manifest v2

Clusterfuzz has detected a corner case where an invalid firmware manifest
containing no main firmware but an OEM firmware would crash the parser.
Add the missing check for this case.

fix another error case for Clusterfuzz, passing absolute file paths in the
manifest would trigger an assert in Append().

BUG=chromium:1196873
BUG=b:184630331
TEST=unit-tests
TEST=verify that the clusterfuzz repro fails without the patch and pass
with it using the following command:
'cros_fuzz --board=zork reproduce --fuzzer firmware_manifest_v2_fuzzer --testcase clusterfuzz-testcase-minimized-firmware_manifest_v2_fuzzer-5960464740450304 --package modemfwd --build-type asan'
TEST=run /usr/libexec/fuzzers/firmware_manifest_v2_fuzzer

Change-Id: I9c4247ce9306eace01c2ee86dbbed3e31f803bbc
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2814678
Tested-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-by: Andrew Lassalle <andrewlassalle@chromium.org>
Commit-Queue: Vincent Palatin <vpalatin@chromium.org>
(cherry picked from commit 2b94342448d24074932fc0dca86f4c88e3210b11)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2843804
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

modemfwd/firmware_manifest_v2.cc

diff --git a/modemfwd/firmware_manifest_v2.cc b/modemfwd/firmware_manifest_v2.cc
index e47b85d..d7011d8 100644
--- a/modemfwd/firmware_manifest_v2.cc
+++ b/modemfwd/firmware_manifest_v2.cc
@@ -43,6 +43,12 @@
     if (!compression.has_value())
       return false;
 
+    if (base::FilePath(main_firmware.filename()).IsAbsolute()) {
+      LOG(ERROR) << "Main firmware should use relative path ("
+                 << main_firmware.filename() << ").";
+      return false;
+    }
+
     main_firmware_infos.emplace(
         main_firmware.version(),
         std::make_unique<FirmwareFileInfo>(
@@ -81,6 +87,12 @@
     if (!compression.has_value())
       return false;
 
+    if (base::FilePath(oem_firmware.filename()).IsAbsolute()) {
+      LOG(ERROR) << "OEM firmware should use relative path ("
+                 << oem_firmware.filename() << ").";
+      return false;
+    }
+
     auto oem_info = std::make_unique<FirmwareFileInfo>(
         directory_path.Append(oem_firmware.filename()), oem_firmware.version(),
         compression.value());
@@ -88,7 +100,8 @@
       for (const std::string& version : oem_firmware.main_firmware_version())
         oem_firmware_infos.emplace(version, oem_info.get());
     } else {
-      oem_firmware_infos.emplace(default_main_entry->version, oem_info.get());
+      if (default_main_entry)
+        oem_firmware_infos.emplace(default_main_entry->version, oem_info.get());
     }
     out_cache->all_files.push_back(std::move(oem_info));
   }
@@ -137,6 +150,12 @@
       return false;
     }
 
+    if (base::FilePath(carrier_firmware.filename()).IsAbsolute()) {
+      LOG(ERROR) << "Carrier firmware should use relative path ("
+                 << carrier_firmware.filename() << ").";
+      return false;
+    }
+
     auto carrier_info = std::make_unique<FirmwareFileInfo>(
         directory_path.Append(carrier_firmware.filename()),
         carrier_firmware.version(), compression.value());