Add checking for sparse file format
Sparse file can come from an untrusted source.
Need more checking to ensure that it is not a malformed
file and would not cause any OOB read access.
Update fuzz test for decoding also.
Test: adb reboot fastboot
fuzzy_fastboot --gtest_filter=Fuzz.Sparse*
fuzzy_fastboot --gtest_filter=Conformance.Sparse*
sparse_fuzzer
Bug: 212705418
Change-Id: I7622df307bb00e59faaba8bb2c67cb474cffed8e
diff --git a/fastboot/device/flashing.cpp b/fastboot/device/flashing.cpp
index 7bef72a..44dc81f 100644
--- a/fastboot/device/flashing.cpp
+++ b/fastboot/device/flashing.cpp
@@ -119,9 +119,11 @@
}
int FlashSparseData(int fd, std::vector<char>& downloaded_data) {
- struct sparse_file* file = sparse_file_import_buf(downloaded_data.data(), true, false);
+ struct sparse_file* file = sparse_file_import_buf(downloaded_data.data(),
+ downloaded_data.size(), true, false);
if (!file) {
- return -ENOENT;
+ // Invalid sparse format
+ return -EINVAL;
}
return sparse_file_callback(file, false, false, WriteCallback, reinterpret_cast<void*>(fd));
}
diff --git a/fastboot/fuzzy_fastboot/main.cpp b/fastboot/fuzzy_fastboot/main.cpp
index 8593adc..055a047 100644
--- a/fastboot/fuzzy_fastboot/main.cpp
+++ b/fastboot/fuzzy_fastboot/main.cpp
@@ -977,6 +977,63 @@
}
}
+TEST_F(Fuzz, SparseZeroBlkSize) {
+ // handcrafted malform sparse file with zero as block size
+ const std::vector<char> buf = {
+ '\x3a', '\xff', '\x26', '\xed', '\x01', '\x00', '\x00', '\x00', '\x1c', '\x00', '\x0c', '\x00',
+ '\x00', '\x00', '\x00', '\x00', '\x01', '\x00', '\x00', '\x00', '\x01', '\x00', '\x00', '\x00',
+ '\x00', '\x00', '\x00', '\x00', '\xc2', '\xca', '\x00', '\x00', '\x01', '\x00', '\x00', '\x00',
+ '\x10', '\x00', '\x00', '\x00', '\x11', '\x22', '\x33', '\x44'
+ };
+
+ ASSERT_EQ(DownloadCommand(buf.size()), SUCCESS) << "Device rejected download command";
+ ASSERT_EQ(SendBuffer(buf), SUCCESS) << "Downloading payload failed";
+
+ // It can either reject this download or reject it during flash
+ if (HandleResponse() != DEVICE_FAIL) {
+ EXPECT_EQ(fb->Flash("userdata"), DEVICE_FAIL)
+ << "Flashing a zero block size in sparse file should fail";
+ }
+}
+
+TEST_F(Fuzz, SparseTrimmed) {
+ // handcrafted malform sparse file which is trimmed
+ const std::vector<char> buf = {
+ '\x3a', '\xff', '\x26', '\xed', '\x01', '\x00', '\x00', '\x00', '\x1c', '\x00', '\x0c', '\x00',
+ '\x00', '\x10', '\x00', '\x00', '\x00', '\x00', '\x08', '\x00', '\x01', '\x00', '\x00', '\x00',
+ '\x00', '\x00', '\x00', '\x00', '\xc1', '\xca', '\x00', '\x00', '\x01', '\x00', '\x00', '\x00',
+ '\x00', '\x00', '\x00', '\x80', '\x11', '\x22', '\x33', '\x44'
+ };
+
+ ASSERT_EQ(DownloadCommand(buf.size()), SUCCESS) << "Device rejected download command";
+ ASSERT_EQ(SendBuffer(buf), SUCCESS) << "Downloading payload failed";
+
+ // It can either reject this download or reject it during flash
+ if (HandleResponse() != DEVICE_FAIL) {
+ EXPECT_EQ(fb->Flash("userdata"), DEVICE_FAIL)
+ << "Flashing a trimmed sparse file should fail";
+ }
+}
+
+TEST_F(Fuzz, SparseInvalidChurk) {
+ // handcrafted malform sparse file with invalid churk
+ const std::vector<char> buf = {
+ '\x3a', '\xff', '\x26', '\xed', '\x01', '\x00', '\x00', '\x00', '\x1c', '\x00', '\x0c', '\x00',
+ '\x00', '\x10', '\x00', '\x00', '\x00', '\x00', '\x08', '\x00', '\x01', '\x00', '\x00', '\x00',
+ '\x00', '\x00', '\x00', '\x00', '\xc1', '\xca', '\x00', '\x00', '\x01', '\x00', '\x00', '\x00',
+ '\x10', '\x00', '\x00', '\x00', '\x11', '\x22', '\x33', '\x44'
+ };
+
+ ASSERT_EQ(DownloadCommand(buf.size()), SUCCESS) << "Device rejected download command";
+ ASSERT_EQ(SendBuffer(buf), SUCCESS) << "Downloading payload failed";
+
+ // It can either reject this download or reject it during flash
+ if (HandleResponse() != DEVICE_FAIL) {
+ EXPECT_EQ(fb->Flash("userdata"), DEVICE_FAIL)
+ << "Flashing a sparse file with invalid churk should fail";
+ }
+}
+
TEST_F(Fuzz, SparseTooManyChunks) {
SparseWrapper sparse(4096, 4096); // 1 block, but we send two chunks that will use 2 blocks
ASSERT_TRUE(*sparse) << "Sparse image creation failed";
diff --git a/libsparse/Android.bp b/libsparse/Android.bp
index 3f9aeb2..02bfee6 100644
--- a/libsparse/Android.bp
+++ b/libsparse/Android.bp
@@ -96,12 +96,14 @@
cc_fuzz {
name: "sparse_fuzzer",
- host_supported: false,
+ host_supported: true,
srcs: [
"sparse_fuzzer.cpp",
],
static_libs: [
"libsparse",
+ "libbase",
+ "libz",
"liblog",
],
}
diff --git a/libsparse/include/sparse/sparse.h b/libsparse/include/sparse/sparse.h
index 2f75349..9f91269 100644
--- a/libsparse/include/sparse/sparse.h
+++ b/libsparse/include/sparse/sparse.h
@@ -244,21 +244,6 @@
int sparse_file_read(struct sparse_file *s, int fd, bool sparse, bool crc);
/**
- * sparse_file_read_buf - read a buffer into a sparse file cookie
- *
- * @s - sparse file cookie
- * @buf - buffer to read from
- * @crc - verify the crc of a file in the Android sparse file format
- *
- * Reads a buffer into a sparse file cookie. The buffer must remain
- * valid until the sparse file cookie is freed. If crc is true, the
- * crc of the sparse file will be verified.
- *
- * Returns 0 on success, negative errno on error.
- */
-int sparse_file_read_buf(struct sparse_file *s, char *buf, bool crc);
-
-/**
* sparse_file_import - import an existing sparse file
*
* @fd - file descriptor to read from
@@ -277,6 +262,7 @@
* sparse_file_import_buf - import an existing sparse file from a buffer
*
* @buf - buffer to read from
+ * @len - length of buffer
* @verbose - print verbose errors while reading the sparse file
* @crc - verify the crc of a file in the Android sparse file format
*
@@ -286,7 +272,7 @@
*
* Returns a new sparse file cookie on success, NULL on error.
*/
-struct sparse_file *sparse_file_import_buf(char* buf, bool verbose, bool crc);
+struct sparse_file* sparse_file_import_buf(char* buf, size_t len, bool verbose, bool crc);
/**
* sparse_file_import_auto - import an existing sparse or normal file
diff --git a/libsparse/output_file.cpp b/libsparse/output_file.cpp
index b2c5407..cb5d730 100644
--- a/libsparse/output_file.cpp
+++ b/libsparse/output_file.cpp
@@ -54,6 +54,8 @@
#define SPARSE_HEADER_LEN (sizeof(sparse_header_t))
#define CHUNK_HEADER_LEN (sizeof(chunk_header_t))
+#define FILL_ZERO_BUFSIZE (2 * 1024 * 1024)
+
#define container_of(inner, outer_t, elem) ((outer_t*)((char*)(inner)-offsetof(outer_t, elem)))
struct output_file_ops {
@@ -391,13 +393,29 @@
ret = out->ops->write(out, data, len);
if (ret < 0) return -1;
if (zero_len) {
- ret = out->ops->write(out, out->zero_buf, zero_len);
- if (ret < 0) return -1;
+ uint64_t len = zero_len;
+ uint64_t write_len;
+ while (len) {
+ write_len = std::min(len, (uint64_t)FILL_ZERO_BUFSIZE);
+ ret = out->ops->write(out, out->zero_buf, write_len);
+ if (ret < 0) {
+ return ret;
+ }
+ len -= write_len;
+ }
}
if (out->use_crc) {
out->crc32 = sparse_crc32(out->crc32, data, len);
- if (zero_len) out->crc32 = sparse_crc32(out->crc32, out->zero_buf, zero_len);
+ if (zero_len) {
+ uint64_t len = zero_len;
+ uint64_t write_len;
+ while (len) {
+ write_len = std::min(len, (uint64_t)FILL_ZERO_BUFSIZE);
+ out->crc32 = sparse_crc32(out->crc32, out->zero_buf, write_len);
+ len -= write_len;
+ }
+ }
}
out->cur_out_ptr += rnd_up_len;
@@ -460,12 +478,12 @@
uint64_t write_len;
/* Initialize fill_buf with the fill_val */
- for (i = 0; i < out->block_size / sizeof(uint32_t); i++) {
+ for (i = 0; i < FILL_ZERO_BUFSIZE / sizeof(uint32_t); i++) {
out->fill_buf[i] = fill_val;
}
while (len) {
- write_len = std::min(len, (uint64_t)out->block_size);
+ write_len = std::min(len, (uint64_t)FILL_ZERO_BUFSIZE);
ret = out->ops->write(out, out->fill_buf, write_len);
if (ret < 0) {
return ret;
@@ -512,13 +530,15 @@
out->crc32 = 0;
out->use_crc = crc;
- out->zero_buf = reinterpret_cast<char*>(calloc(block_size, 1));
+ // don't use sparse format block size as it can takes up to 32GB
+ out->zero_buf = reinterpret_cast<char*>(calloc(FILL_ZERO_BUFSIZE, 1));
if (!out->zero_buf) {
error_errno("malloc zero_buf");
return -ENOMEM;
}
- out->fill_buf = reinterpret_cast<uint32_t*>(calloc(block_size, 1));
+ // don't use sparse format block size as it can takes up to 32GB
+ out->fill_buf = reinterpret_cast<uint32_t*>(calloc(FILL_ZERO_BUFSIZE, 1));
if (!out->fill_buf) {
error_errno("malloc fill_buf");
ret = -ENOMEM;
diff --git a/libsparse/sparse_fuzzer.cpp b/libsparse/sparse_fuzzer.cpp
index 42f331f..235d15d 100644
--- a/libsparse/sparse_fuzzer.cpp
+++ b/libsparse/sparse_fuzzer.cpp
@@ -1,16 +1,27 @@
#include "include/sparse/sparse.h"
-extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
- if (size < 2 * sizeof(wchar_t)) return 0;
+static volatile int count;
- int64_t blocksize = 4096;
- struct sparse_file* file = sparse_file_new(size, blocksize);
- if (!file) {
+int WriteCallback(void* priv __attribute__((__unused__)), const void* data, size_t len) {
+ if (!data) {
+ return 0;
+ }
+ if (len == 0) {
return 0;
}
- unsigned int block = 1;
- sparse_file_add_data(file, &data, size, block);
- sparse_file_destroy(file);
+ const char* p = (const char*)data;
+ // Just to make sure the data is accessible
+ // We only check the head and tail to save time
+ count += *p;
+ count += *(p+len-1);
return 0;
}
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
+ struct sparse_file* file = sparse_file_import_buf((char*)data, size, true, false);
+ if (!file) {
+ return 0;
+ }
+ return sparse_file_callback(file, false, false, WriteCallback, nullptr);
+}
diff --git a/libsparse/sparse_read.cpp b/libsparse/sparse_read.cpp
index c4c1823..0f39172 100644
--- a/libsparse/sparse_read.cpp
+++ b/libsparse/sparse_read.cpp
@@ -58,14 +58,15 @@
class SparseFileSource {
public:
- /* Seeks the source ahead by the given offset. */
- virtual void Seek(int64_t offset) = 0;
+ /* Seeks the source ahead by the given offset.
+ * Return 0 if successful. */
+ virtual int Seek(int64_t offset) = 0;
/* Return the current offset. */
virtual int64_t GetOffset() = 0;
- /* Set the current offset. Return 0 if successful. */
- virtual int SetOffset(int64_t offset) = 0;
+ /* Rewind to beginning. Return 0 if successful. */
+ virtual int Rewind() = 0;
/* Adds the given length from the current offset of the source to the file at the given block.
* Return 0 if successful. */
@@ -88,12 +89,14 @@
SparseFileFdSource(int fd) : fd(fd) {}
~SparseFileFdSource() override {}
- void Seek(int64_t off) override { lseek64(fd, off, SEEK_CUR); }
+ int Seek(int64_t off) override {
+ return lseek64(fd, off, SEEK_CUR) != -1 ? 0 : -errno;
+ }
int64_t GetOffset() override { return lseek64(fd, 0, SEEK_CUR); }
- int SetOffset(int64_t offset) override {
- return lseek64(fd, offset, SEEK_SET) == offset ? 0 : -errno;
+ int Rewind() override {
+ return lseek64(fd, 0, SEEK_SET) == 0 ? 0 : -errno;
}
int AddToSparseFile(struct sparse_file* s, int64_t len, unsigned int block) override {
@@ -120,39 +123,74 @@
class SparseFileBufSource : public SparseFileSource {
private:
+ char* buf_start;
+ char* buf_end;
char* buf;
int64_t offset;
+ int AccessOkay(int64_t len) {
+ if (len <= 0) return -EINVAL;
+ if (buf < buf_start) return -EOVERFLOW;
+ if (buf >= buf_end) return -EOVERFLOW;
+ if (len > buf_end - buf) return -EOVERFLOW;
+
+ return 0;
+ }
+
public:
- SparseFileBufSource(char* buf) : buf(buf), offset(0) {}
+ SparseFileBufSource(char* buf, uint64_t len) {
+ this->buf = buf;
+ this->offset = 0;
+ this->buf_start = buf;
+ this->buf_end = buf + len;
+ }
~SparseFileBufSource() override {}
- void Seek(int64_t off) override {
+ int Seek(int64_t off) override {
+ int ret = AccessOkay(off);
+ if (ret < 0) {
+ return ret;
+ }
buf += off;
offset += off;
+ return 0;
}
int64_t GetOffset() override { return offset; }
- int SetOffset(int64_t off) override {
- buf += off - offset;
- offset = off;
+ int Rewind() override {
+ buf = buf_start;
+ offset = 0;
return 0;
}
int AddToSparseFile(struct sparse_file* s, int64_t len, unsigned int block) override {
+ int ret = AccessOkay(len);
+ if (ret < 0) {
+ return ret;
+ }
return sparse_file_add_data(s, buf, len, block);
}
int ReadValue(void* ptr, int len) override {
+ int ret = AccessOkay(len);
+ if (ret < 0) {
+ return ret;
+ }
memcpy(ptr, buf, len);
- Seek(len);
+ buf += len;
+ offset += len;
return 0;
}
int GetCrc32(uint32_t* crc32, int64_t len) override {
+ int ret = AccessOkay(len);
+ if (ret < 0) {
+ return ret;
+ }
*crc32 = sparse_crc32(*crc32, buf, len);
- Seek(len);
+ buf += len;
+ offset += len;
return 0;
}
};
@@ -175,7 +213,7 @@
SparseFileSource* source, unsigned int blocks, unsigned int block,
uint32_t* crc32) {
int ret;
- int64_t len = blocks * s->block_size;
+ int64_t len = (int64_t)blocks * s->block_size;
if (chunk_size % s->block_size != 0) {
return -EINVAL;
@@ -196,7 +234,10 @@
return ret;
}
} else {
- source->Seek(len);
+ ret = source->Seek(len);
+ if (ret < 0) {
+ return ret;
+ }
}
return 0;
@@ -379,7 +420,10 @@
/* Skip the remaining bytes in a header that is longer than
* we expected.
*/
- source->Seek(sparse_header.file_hdr_sz - SPARSE_HEADER_LEN);
+ ret = source->Seek(sparse_header.file_hdr_sz - SPARSE_HEADER_LEN);
+ if (ret < 0) {
+ return ret;
+ }
}
for (i = 0; i < sparse_header.total_chunks; i++) {
@@ -392,7 +436,10 @@
/* Skip the remaining bytes in a header that is longer than
* we expected.
*/
- source->Seek(sparse_header.chunk_hdr_sz - CHUNK_HEADER_LEN);
+ ret = source->Seek(sparse_header.chunk_hdr_sz - CHUNK_HEADER_LEN);
+ if (ret < 0) {
+ return ret;
+ }
}
ret = process_chunk(s, source, sparse_header.chunk_hdr_sz, &chunk_header, cur_block, crc_ptr);
@@ -474,11 +521,6 @@
}
}
-int sparse_file_read_buf(struct sparse_file* s, char* buf, bool crc) {
- SparseFileBufSource source(buf);
- return sparse_file_read_sparse(s, &source, crc);
-}
-
static struct sparse_file* sparse_file_import_source(SparseFileSource* source, bool verbose,
bool crc) {
int ret;
@@ -510,6 +552,14 @@
return nullptr;
}
+ if (!sparse_header.blk_sz || (sparse_header.blk_sz % 4)) {
+ return nullptr;
+ }
+
+ if (!sparse_header.total_blks) {
+ return nullptr;
+ }
+
len = (int64_t)sparse_header.total_blks * sparse_header.blk_sz;
s = sparse_file_new(sparse_header.blk_sz, len);
if (!s) {
@@ -517,7 +567,7 @@
return nullptr;
}
- ret = source->SetOffset(0);
+ ret = source->Rewind();
if (ret < 0) {
verbose_error(verbose, ret, "seeking");
sparse_file_destroy(s);
@@ -540,8 +590,8 @@
return sparse_file_import_source(&source, verbose, crc);
}
-struct sparse_file* sparse_file_import_buf(char* buf, bool verbose, bool crc) {
- SparseFileBufSource source(buf);
+struct sparse_file* sparse_file_import_buf(char* buf, size_t len, bool verbose, bool crc) {
+ SparseFileBufSource source(buf, len);
return sparse_file_import_source(&source, verbose, crc);
}