Do not unwrap when getting decoder and encoder

Or else things will crash mysteriously when unexpected input occurs

native/src/boot/bootimg.cpp

@@ -53,6 +53,19 @@ static bool check_env(const char *name) {
     return val != nullptr && val == "true"sv;
 }
 
+static bool guess_lzma(const uint8_t *buf, size_t len) {
+    // 0     : (pb * 5 + lp) * 9 + lc
+    // 1 - 4 : dict size, must be 2^n
+    // 5 - 12: all 0xFF
+    if (len <= 13) return false;
+    if (memcmp(buf, "\x5d", 1) != 0) return false;
+    uint32_t dict_sz = 0;
+    memcpy(&dict_sz, buf + 1, sizeof(dict_sz));
+    if (dict_sz == 0 || (dict_sz & (dict_sz - 1)) != 0) return false;
+    if (memcmp(buf + 5, "\xff\xff\xff\xff\xff\xff\xff\xff", 8) != 0) return false;
+    return true;
+}
+
 FileFormat check_fmt(const void *buf, size_t len) {
     if (CHECKED_MATCH(CHROMEOS_MAGIC)) {
         return FileFormat::CHROMEOS;
@@ -66,8 +79,7 @@ FileFormat check_fmt(const void *buf, size_t len) {
         return FileFormat::LZOP;
     } else if (CHECKED_MATCH(XZ_MAGIC)) {
         return FileFormat::XZ;
-    } else if (len >= 13 && memcmp(buf, "\x5d\x00\x00", 3) == 0
-            && (((char *)buf)[12] == '\xff' || ((char *)buf)[12] == '\x00')) {
+    } else if (guess_lzma(static_cast<const uint8_t *>(buf), len)) {
         return FileFormat::LZMA;
     } else if (CHECKED_MATCH(BZIP_MAGIC)) {
         return FileFormat::BZIP2;

native/src/boot/compress.rs

@@ -1,8 +1,6 @@
 use crate::ffi::{FileFormat, check_fmt};
 use base::nix::fcntl::OFlag;
-use base::{
-    Chunker, FileOrStd, LoggedResult, ReadExt, ResultExt, Utf8CStr, Utf8CString, WriteExt, log_err,
-};
+use base::{Chunker, FileOrStd, LoggedResult, ReadExt, Utf8CStr, Utf8CString, WriteExt, log_err};
 use bzip2::Compression as BzCompression;
 use bzip2::read::BzDecoder;
 use bzip2::write::BzEncoder;
@@ -218,16 +216,21 @@ impl<R: Read> Read for LZ4BlockDecoder<R> {
 
 // Top-level APIs
 
-pub fn get_encoder<'a, W: Write + 'a>(format: FileFormat, w: W) -> Box<dyn WriteFinish<W> + 'a> {
-    match format {
+pub fn get_encoder<'a, W: Write + 'a>(
+    format: FileFormat,
+    w: W,
+) -> std::io::Result<Box<dyn WriteFinish<W> + 'a>> {
+    Ok(match format {
         FileFormat::XZ => {
             let mut opt = XzOptions::with_preset(9);
             opt.set_check_sum_type(CheckType::Crc32);
-            Box::new(XzWriter::new(w, opt).unwrap())
-        }
-        FileFormat::LZMA => {
-            Box::new(LzmaWriter::new_use_header(w, &LzmaOptions::with_preset(9), None).unwrap())
+            Box::new(XzWriter::new(w, opt)?)
         }
+        FileFormat::LZMA => Box::new(LzmaWriter::new_use_header(
+            w,
+            &LzmaOptions::with_preset(9),
+            None,
+        )?),
         FileFormat::BZIP2 => Box::new(BzEncoder::new(w, BzCompression::best())),
         FileFormat::LZ4 => {
             let encoder = LZ4FrameEncoderBuilder::new()
@@ -237,8 +240,7 @@ pub fn get_encoder<'a, W: Write + 'a>(format: FileFormat, w: W) -> Box<dyn Write
                 .block_checksum(BlockChecksum::BlockChecksumEnabled)
                 .level(9)
                 .auto_flush(true)
-                .build(w)
-                .unwrap();
+                .build(w)?;
             Box::new(encoder)
         }
         FileFormat::LZ4_LEGACY => Box::new(LZ4BlockEncoder::new(w, false)),
@@ -250,23 +252,26 @@ pub fn get_encoder<'a, W: Write + 'a>(format: FileFormat, w: W) -> Box<dyn Write
                 maximum_block_splits: 1,
                 ..Default::default()
             };
-            Box::new(ZopFliEncoder::new_buffered(opt, BlockType::Dynamic, w).unwrap())
+            Box::new(ZopFliEncoder::new_buffered(opt, BlockType::Dynamic, w)?)
         }
         FileFormat::GZIP => Box::new(GzEncoder::new(w, GzCompression::best())),
         _ => unreachable!(),
-    }
+    })
 }
 
-pub fn get_decoder<'a, R: Read + 'a>(format: FileFormat, r: R) -> Box<dyn Read + 'a> {
-    match format {
+pub fn get_decoder<'a, R: Read + 'a>(
+    format: FileFormat,
+    r: R,
+) -> std::io::Result<Box<dyn Read + 'a>> {
+    Ok(match format {
         FileFormat::XZ => Box::new(XzReader::new(r, true)),
-        FileFormat::LZMA => Box::new(LzmaReader::new_mem_limit(r, u32::MAX, None).unwrap()),
+        FileFormat::LZMA => Box::new(LzmaReader::new_mem_limit(r, u32::MAX, None)?),
         FileFormat::BZIP2 => Box::new(BzDecoder::new(r)),
-        FileFormat::LZ4 => Box::new(LZ4FrameDecoder::new(r).unwrap()),
+        FileFormat::LZ4 => Box::new(LZ4FrameDecoder::new(r)?),
         FileFormat::LZ4_LG | FileFormat::LZ4_LEGACY => Box::new(LZ4BlockDecoder::new(r)),
         FileFormat::ZOPFLI | FileFormat::GZIP => Box::new(MultiGzDecoder::new(r)),
         _ => unreachable!(),
-    }
+    })
 }
 
 // C++ FFI
@@ -274,9 +279,9 @@ pub fn get_decoder<'a, R: Read + 'a>(format: FileFormat, r: R) -> Box<dyn Read +
 pub fn compress_bytes(format: FileFormat, in_bytes: &[u8], out_fd: RawFd) {
     let mut out_file = unsafe { ManuallyDrop::new(File::from_raw_fd(out_fd)) };
 
-    let mut encoder = get_encoder(format, out_file.deref_mut());
     let _: LoggedResult<()> = try {
-        encoder.write_all(in_bytes)?;
+        let mut encoder = get_encoder(format, out_file.deref_mut())?;
+        std::io::copy(&mut Cursor::new(in_bytes), encoder.deref_mut())?;
         encoder.finish()?;
     };
 }
@@ -284,8 +289,10 @@ pub fn compress_bytes(format: FileFormat, in_bytes: &[u8], out_fd: RawFd) {
 pub fn decompress_bytes(format: FileFormat, in_bytes: &[u8], out_fd: RawFd) {
     let mut out_file = unsafe { ManuallyDrop::new(File::from_raw_fd(out_fd)) };
 
-    let mut decoder = get_decoder(format, in_bytes);
-    std::io::copy(decoder.as_mut(), out_file.deref_mut()).log_ok();
+    let _: LoggedResult<()> = try {
+        let mut decoder = get_decoder(format, in_bytes)?;
+        std::io::copy(decoder.as_mut(), out_file.deref_mut())?;
+    };
 }
 
 // Command-line entry points
@@ -341,7 +348,7 @@ pub(crate) fn decompress_cmd(infile: &Utf8CStr, outfile: Option<&Utf8CStr>) -> L
         FileOrStd::File(outfile.create(OFlag::O_WRONLY | OFlag::O_TRUNC, 0o644)?)
     };
 
-    let mut decoder = get_decoder(format, Cursor::new(buf).chain(input.as_file()));
+    let mut decoder = get_decoder(format, Cursor::new(buf).chain(input.as_file()))?;
     std::io::copy(decoder.as_mut(), &mut output.as_file())?;
 
     if rm_in {
@@ -384,7 +391,7 @@ pub(crate) fn compress_cmd(
         FileOrStd::File(outfile)
     };
 
-    let mut encoder = get_encoder(method, output.as_file());
+    let mut encoder = get_encoder(method, output.as_file())?;
     std::io::copy(&mut input.as_file(), encoder.as_mut())?;
     encoder.finish()?;
 

native/src/boot/cpio.rs

@@ -691,8 +691,8 @@ impl CpioEntry {
         if self.mode & S_IFMT != S_IFREG {
             return false;
         }
-        let mut encoder = get_encoder(FileFormat::XZ, Vec::new());
         let Ok(data): std::io::Result<Vec<u8>> = (try {
+            let mut encoder = get_encoder(FileFormat::XZ, Vec::new())?;
             encoder.write_all(&self.data)?;
             encoder.finish()?
         }) else {
@@ -710,7 +710,7 @@ impl CpioEntry {
         }
 
         let Ok(data): std::io::Result<Vec<u8>> = (try {
-            let mut decoder = get_decoder(FileFormat::XZ, Cursor::new(&self.data));
+            let mut decoder = get_decoder(FileFormat::XZ, Cursor::new(&self.data))?;
             let mut data = Vec::new();
             std::io::copy(decoder.as_mut(), &mut data)?;
             data

native/src/boot/payload.rs

@@ -164,8 +164,8 @@ pub fn extract_boot_from_payload(
                 out_file.seek(SeekFrom::Start(out_offset))?;
                 let fmt = check_fmt(data);
 
-                let mut decoder = get_decoder(fmt, Cursor::new(data));
                 let Ok(_): std::io::Result<()> = (try {
+                    let mut decoder = get_decoder(fmt, Cursor::new(data))?;
                     std::io::copy(decoder.as_mut(), &mut out_file)?;
                 }) else {
                     return Err(bad_payload!("decompression failed"));

scripts/avd.sh

@@ -32,17 +32,17 @@ case $(uname -m) in
 esac
 
 cleanup() {
-  pkill -INT -P $$
-  wait
-  trap - EXIT
   rm -f magisk_*.img
   "$avd" delete avd -n test
-  exit 1
 }
 
 test_error() {
+  trap - EXIT
   print_error "! An error occurred"
+  pkill -INT -P $$
+  wait
   cleanup
+  exit 1
 }
 
 wait_for_boot() {
@@ -72,13 +72,14 @@ wait_emu() {
 
 dump_vars() {
   local val
-  for name in $@; do
+  for name in $@ emu_args; do
     eval val=\$$name
     echo $name=\"$val\"\;
   done
 }
 
 resolve_vars() {
+  set +x
   local arg_list="$1"
   local ver=$2
   local type=$3
@@ -138,8 +139,14 @@ dl_emu() {
 
 setup_emu() {
   local avd_pkg=$1
+  local ver=$2
   dl_emu $avd_pkg
   echo no | "$avd" create avd -f -n test -k $avd_pkg
+
+  # avdmanager is outdated, it might not set the proper target
+  local ini=$ANDROID_AVD_HOME/test.ini
+  sed "s:^target\s*=.*:target=android-$ver:g" $ini > $ini.new
+  mv $ini.new $ini
 }
 
 test_emu() {
@@ -169,16 +176,15 @@ test_emu() {
 }
 
 test_main() {
-  local avd_pkg ramdisk vars
-  vars=$(resolve_vars "emu_args avd_pkg ramdisk" $1 $2)
-  eval $vars
+  local ver avd_pkg ramdisk
+  eval $(resolve_vars "ver avd_pkg ramdisk" $1 $2)
 
   # Specify an explicit port so that tests can run with other emulators running at the same time
   local emu_port=5682
   emu_args="$emu_args -port $emu_port"
   export ANDROID_SERIAL="emulator-$emu_port"
 
-  setup_emu "$avd_pkg"
+  setup_emu "$avd_pkg" $ver
 
   # Restart ADB daemon just in case
   adb kill-server
@@ -211,24 +217,21 @@ test_main() {
     test_emu release
   fi
 
-  # Cleanup
-  rm -f magisk_*.img
-  "$avd" delete avd -n test
+  cleanup
 }
 
 run_main() {
-  local avd_pkg vars
-  vars=$(resolve_vars "emu_args avd_pkg" $1 $2)
-  eval $vars
-  setup_emu "$avd_pkg"
+  local ver avd_pkg
+  eval $(resolve_vars "ver avd_pkg" $1 $2)
+  setup_emu "$avd_pkg" $ver
   print_title "* Launching $avd_pkg"
   "$emu" @test $emu_args 2>/dev/null
+  cleanup
 }
 
 dl_main() {
-  local avd_pkg vars
-  vars=$(resolve_vars "avd_pkg" $1 $2)
-  eval $vars
+  local avd_pkg
+  eval $(resolve_vars "avd_pkg" $1 $2)
   print_title "* Downloading $avd_pkg"
   dl_emu "$avd_pkg"
 }