From 7df23ceb748308242573b819c12fd29fe8317b09 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Mon, 10 May 2021 18:38:30 -0700 Subject: [PATCH] Prevent undefined behavior in magiskboot --- native/jni/magiskboot/bootimg.cpp | 7 +--- native/jni/magiskboot/compress.cpp | 55 +++++++++++++++--------------- native/jni/magiskboot/dtb.cpp | 2 -- native/jni/magiskboot/hexpatch.cpp | 2 +- native/jni/magiskboot/main.cpp | 6 ---- native/jni/magiskboot/pattern.cpp | 2 -- native/jni/magiskboot/ramdisk.cpp | 2 +- 7 files changed, 31 insertions(+), 45 deletions(-) diff --git a/native/jni/magiskboot/bootimg.cpp b/native/jni/magiskboot/bootimg.cpp index b41c9ca7c..cf5ea79f2 100644 --- a/native/jni/magiskboot/bootimg.cpp +++ b/native/jni/magiskboot/bootimg.cpp @@ -1,12 +1,7 @@ -#include -#include -#include -#include -#include -#include #include #include +#include #include #include #include diff --git a/native/jni/magiskboot/compress.cpp b/native/jni/magiskboot/compress.cpp index c43e05a41..157d95968 100644 --- a/native/jni/magiskboot/compress.cpp +++ b/native/jni/magiskboot/compress.cpp @@ -1,8 +1,3 @@ -#include -#include -#include -#include -#include #include #include @@ -56,7 +51,8 @@ protected: ENCODE } mode; - gz_strm(mode_t mode, stream_ptr &&base) : cpr_stream(std::move(base)), mode(mode) { + gz_strm(mode_t mode, stream_ptr &&base) : + cpr_stream(std::move(base)), mode(mode), strm{}, outbuf{0} { switch(mode) { case DECODE: inflateInit2(&strm, 15 | 16); @@ -131,7 +127,8 @@ protected: ENCODE } mode; - bz_strm(mode_t mode, stream_ptr &&base) : cpr_stream(std::move(base)), mode(mode) { + bz_strm(mode_t mode, stream_ptr &&base) : + cpr_stream(std::move(base)), mode(mode), strm{}, outbuf{0} { switch(mode) { case DECODE: BZ2_bzDecompressInit(&strm, 0, 0); @@ -200,8 +197,8 @@ protected: ENCODE_LZMA } mode; - lzma_strm(mode_t mode, stream_ptr &&base) - : cpr_stream(std::move(base)), mode(mode), strm(LZMA_STREAM_INIT) { + lzma_strm(mode_t mode, stream_ptr &&base) : + cpr_stream(std::move(base)), mode(mode), strm(LZMA_STREAM_INIT), outbuf{0} { lzma_options_lzma opt; // Initialize preset @@ -211,18 +208,21 @@ protected: { .id = LZMA_VLI_UNKNOWN, .options = nullptr }, }; - lzma_ret ret; + lzma_ret code; switch(mode) { case DECODE: - ret = lzma_auto_decoder(&strm, UINT64_MAX, 0); + code = lzma_auto_decoder(&strm, UINT64_MAX, 0); break; case ENCODE_XZ: - ret = lzma_stream_encoder(&strm, filters, LZMA_CHECK_CRC32); + code = lzma_stream_encoder(&strm, filters, LZMA_CHECK_CRC32); break; case ENCODE_LZMA: - ret = lzma_alone_encoder(&strm, &opt); + code = lzma_alone_encoder(&strm, &opt); break; } + if (code != LZMA_OK) { + LOGE("LZMA initialization failed (%d)\n", code); + } } private: @@ -264,7 +264,8 @@ public: class LZ4F_decoder : public cpr_stream { public: - explicit LZ4F_decoder(stream_ptr &&base) : cpr_stream(std::move(base)), outbuf(nullptr) { + explicit LZ4F_decoder(stream_ptr &&base) : + cpr_stream(std::move(base)), ctx(nullptr), outbuf(nullptr), outCapacity(0) { LZ4F_createDecompressionContext(&ctx, LZ4F_VERSION); } @@ -319,8 +320,8 @@ private: class LZ4F_encoder : public cpr_stream { public: - explicit LZ4F_encoder(stream_ptr &&base) - : cpr_stream(std::move(base)), outbuf(nullptr), outCapacity(0) { + explicit LZ4F_encoder(stream_ptr &&base) : + cpr_stream(std::move(base)), ctx(nullptr), outbuf(nullptr), outCapacity(0) { LZ4F_createCompressionContext(&ctx, LZ4F_VERSION); } @@ -362,14 +363,14 @@ private: int write_header() { LZ4F_preferences_t prefs { - .autoFlush = 1, - .compressionLevel = 9, .frameInfo = { - .blockMode = LZ4F_blockIndependent, .blockSizeID = LZ4F_max4MB, + .blockMode = LZ4F_blockIndependent, + .contentChecksumFlag = LZ4F_contentChecksumEnabled, .blockChecksumFlag = LZ4F_noBlockChecksum, - .contentChecksumFlag = LZ4F_contentChecksumEnabled - } + }, + .compressionLevel = 9, + .autoFlush = 1, }; outCapacity = LZ4F_compressBound(BLOCK_SZ, &prefs); outbuf = new uint8_t[outCapacity]; @@ -380,9 +381,9 @@ private: class LZ4_decoder : public cpr_stream { public: - explicit LZ4_decoder(stream_ptr &&base) - : cpr_stream(std::move(base)), out_buf(new char[LZ4_UNCOMPRESSED]), - buf(new char[LZ4_COMPRESSED]), init(false), block_sz(0), buf_off(0) {} + explicit LZ4_decoder(stream_ptr &&base) : + cpr_stream(std::move(base)), out_buf(new char[LZ4_UNCOMPRESSED]), + buf(new char[LZ4_COMPRESSED]), init(false), block_sz(0), buf_off(0) {} ~LZ4_decoder() override { delete[] out_buf; @@ -447,9 +448,9 @@ private: class LZ4_encoder : public cpr_stream { public: - explicit LZ4_encoder(stream_ptr &&base, bool lg) - : cpr_stream(std::move(base)), outbuf(new char[LZ4_COMPRESSED]), - buf(new char[LZ4_UNCOMPRESSED]), init(false), lg(lg), buf_off(0), in_total(0) {} + explicit LZ4_encoder(stream_ptr &&base, bool lg) : + cpr_stream(std::move(base)), outbuf(new char[LZ4_COMPRESSED]), + buf(new char[LZ4_UNCOMPRESSED]), init(false), lg(lg), buf_off(0), in_total(0) {} int write(const void *in, size_t size) override { int ret = 0; diff --git a/native/jni/magiskboot/dtb.cpp b/native/jni/magiskboot/dtb.cpp index 24bbbad45..7f422c1ea 100644 --- a/native/jni/magiskboot/dtb.cpp +++ b/native/jni/magiskboot/dtb.cpp @@ -1,5 +1,3 @@ -#include -#include #include #include #include diff --git a/native/jni/magiskboot/hexpatch.cpp b/native/jni/magiskboot/hexpatch.cpp index 15dc8b097..b32d99553 100644 --- a/native/jni/magiskboot/hexpatch.cpp +++ b/native/jni/magiskboot/hexpatch.cpp @@ -34,7 +34,7 @@ int hexpatch(const char *image, const char *from, const char *to) { curr = static_cast(memmem(curr, end - curr, pattern.data(), pattern.size())); if (curr == nullptr) return patched; - fprintf(stderr, "Patch @ %08X [%s] -> [%s]\n", curr - buf, from, to); + fprintf(stderr, "Patch @ %08X [%s] -> [%s]\n", (unsigned)(curr - buf), from, to); memset(curr, 0, pattern.size()); memcpy(curr, patch.data(), patch.size()); patched = 0; diff --git a/native/jni/magiskboot/main.cpp b/native/jni/magiskboot/main.cpp index 00a129a25..f78ce163b 100644 --- a/native/jni/magiskboot/main.cpp +++ b/native/jni/magiskboot/main.cpp @@ -1,9 +1,3 @@ -#include -#include -#include -#include -#include - #include #include diff --git a/native/jni/magiskboot/pattern.cpp b/native/jni/magiskboot/pattern.cpp index df0cdc0ac..16aed269e 100644 --- a/native/jni/magiskboot/pattern.cpp +++ b/native/jni/magiskboot/pattern.cpp @@ -1,5 +1,3 @@ -#include - #include #include "magiskboot.hpp" diff --git a/native/jni/magiskboot/ramdisk.cpp b/native/jni/magiskboot/ramdisk.cpp index 3347b43a0..ed61a79d0 100644 --- a/native/jni/magiskboot/ramdisk.cpp +++ b/native/jni/magiskboot/ramdisk.cpp @@ -31,7 +31,7 @@ public: bool check_env(const char *name) { const char *val = getenv(name); - return val ? val == "true"sv : false; + return val != nullptr && val == "true"sv; } void magisk_cpio::patch() {