From 0f88d6502fc35e5a9255d85b7442061f25d50d76 Mon Sep 17 00:00:00 2001
From: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com>
Date: Fri, 24 Sep 2021 17:46:00 +0200
Subject: [PATCH] Storage panic from oss-fuzz (#385)

* fixes the fuzzer flaw and adds tests for it

* out of bounds instead of empty okay
---
 src/embedded_flash/buffer_upgrade.rs | 67 ++++++++++++++++++++++++++++
 src/embedded_flash/syscall.rs        | 14 +++---
 2 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/src/embedded_flash/buffer_upgrade.rs b/src/embedded_flash/buffer_upgrade.rs
index 780d3c3..8f4bc6a 100644
--- a/src/embedded_flash/buffer_upgrade.rs
+++ b/src/embedded_flash/buffer_upgrade.rs
@@ -39,6 +39,9 @@ impl BufferUpgradeStorage {
 
 impl UpgradeStorage for BufferUpgradeStorage {
     fn read_partition(&self, offset: usize, length: usize) -> StorageResult<&[u8]> {
+        if length == 0 {
+            return Err(StorageError::OutOfBounds);
+        }
         let partition_range = ModRange::new(0, self.partition.len());
         if partition_range.contains_range(&ModRange::new(offset, length)) {
             Ok(&self.partition[offset..][..length])
@@ -48,6 +51,9 @@ impl UpgradeStorage for BufferUpgradeStorage {
     }
 
     fn write_partition(&mut self, offset: usize, data: &[u8]) -> StorageResult<()> {
+        if data.is_empty() {
+            return Err(StorageError::OutOfBounds);
+        }
         let partition_range = ModRange::new(0, self.partition.len());
         if partition_range.contains_range(&ModRange::new(offset, data.len())) {
             self.partition[offset..][..data.len()].copy_from_slice(data);
@@ -79,3 +85,64 @@ impl UpgradeStorage for BufferUpgradeStorage {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn read_write_partition() {
+        let mut storage = BufferUpgradeStorage::new().unwrap();
+        assert_eq!(storage.read_partition(0, 2).unwrap(), &[0xFF, 0xFF]);
+        assert!(storage.write_partition(1, &[0x88, 0x88]).is_ok());
+        assert_eq!(storage.read_partition(0, 2).unwrap(), &[0xFF, 0x88]);
+        assert_eq!(
+            storage.write_partition(PARTITION_LENGTH - 1, &[0x88, 0x88]),
+            Err(StorageError::OutOfBounds)
+        );
+        assert_eq!(
+            storage.read_partition(PARTITION_LENGTH - 2, 2).unwrap(),
+            &[0xFF, 0xFF]
+        );
+        assert_eq!(
+            storage.read_partition(PARTITION_LENGTH - 1, 2),
+            Err(StorageError::OutOfBounds)
+        );
+        assert_eq!(
+            storage.write_partition(4, &[]),
+            Err(StorageError::OutOfBounds)
+        );
+        assert_eq!(
+            storage.write_partition(PARTITION_LENGTH + 4, &[]),
+            Err(StorageError::OutOfBounds)
+        );
+        assert_eq!(storage.read_partition(4, 0), Err(StorageError::OutOfBounds));
+        assert_eq!(
+            storage.read_partition(PARTITION_LENGTH + 4, 0),
+            Err(StorageError::OutOfBounds)
+        );
+    }
+
+    #[test]
+    fn partition_slice() {
+        let storage = BufferUpgradeStorage::new().unwrap();
+        assert_eq!(storage.partition_address(), 0x60000);
+        assert_eq!(storage.partition_length(), PARTITION_LENGTH);
+    }
+
+    #[test]
+    fn read_write_metadata() {
+        let mut storage = BufferUpgradeStorage::new().unwrap();
+        assert_eq!(storage.read_metadata().unwrap(), &[0xFF; METADATA_LENGTH]);
+        assert!(storage.write_metadata(&[0x88, 0x88]).is_ok());
+        assert_eq!(
+            storage.write_metadata(&[0x88; METADATA_LENGTH + 1]),
+            Err(StorageError::OutOfBounds)
+        );
+        let new_metadata = storage.read_metadata().unwrap();
+        assert_eq!(&new_metadata[0..2], &[0x88, 0x88]);
+        assert_eq!(&new_metadata[2..], &[0xFF; METADATA_LENGTH - 2]);
+        assert!(storage.write_metadata(&[]).is_ok());
+        assert_eq!(storage.read_metadata().unwrap(), &[0xFF; METADATA_LENGTH]);
+    }
+}
diff --git a/src/embedded_flash/syscall.rs b/src/embedded_flash/syscall.rs
index 0bd5243..d5dac9c 100644
--- a/src/embedded_flash/syscall.rs
+++ b/src/embedded_flash/syscall.rs
@@ -289,6 +289,9 @@ impl SyscallUpgradeStorage {
 
 impl UpgradeStorage for SyscallUpgradeStorage {
     fn read_partition(&self, offset: usize, length: usize) -> StorageResult<&[u8]> {
+        if length == 0 {
+            return Err(StorageError::OutOfBounds);
+        }
         let address = self.partition.start() + offset;
         if self
             .partition
@@ -301,14 +304,15 @@ impl UpgradeStorage for SyscallUpgradeStorage {
     }
 
     fn write_partition(&mut self, offset: usize, data: &[u8]) -> StorageResult<()> {
+        if data.is_empty() {
+            return Err(StorageError::OutOfBounds);
+        }
         let address = self.partition.start() + offset;
-        if self
-            .partition
-            .contains_range(&ModRange::new(address, data.len()))
-        {
+        let write_range = ModRange::new(address, data.len());
+        if self.partition.contains_range(&write_range) {
             // Erases all pages that have their first byte in the write range.
             // Since we expect calls in order, we don't want to erase half-written pages.
-            for address in ModRange::new(address, data.len()).aligned_iter(self.page_size) {
+            for address in write_range.aligned_iter(self.page_size) {
                 erase_page(address, self.page_size)?;
             }
             write_slice(address, data)