diff options
author | Damien George <damien@micropython.org> | 2025-02-24 23:14:48 +1100 |
---|---|---|
committer | Damien George <damien@micropython.org> | 2025-02-26 16:11:19 +1100 |
commit | 14ba32bb205fa1f7d6ac456879b08aadb4e1aaf7 (patch) | |
tree | e0993602c59c6b4c20bd81fb4fea5dcc5e64ed78 /tests | |
parent | e3101ce1b3782955ea2d103922f0dc19bc0331f6 (diff) | |
download | micropython-14ba32bb205fa1f7d6ac456879b08aadb4e1aaf7.tar.gz micropython-14ba32bb205fa1f7d6ac456879b08aadb4e1aaf7.zip |
extmod/vfs_rom: Add bounds checking for all filesystem accesses.
Testing with ROMFS shows that it is relatively easy to end up with a
corrupt filesystem on the device -- eg due to the ROMFS deploy process
stopping half way through -- which could lead to hard crashes. Notably,
there can be boot loops trying to mount a corrupt filesystem, crashes when
importing modules like `os` that first scan the filesystem for `os.py`, and
crashing when deploying a new ROMFS in certain cases because the old one is
removed while still mounted.
The main problem is that `mp_decode_uint()` has an loop that keeps going as
long as it reads 0xff byte values, which can happen in the case of erased
and unwritten flash.
This commit adds full bounds checking in the new `mp_decode_uint_checked()`
function, and that makes all ROMFS filesystem accesses robust.
Signed-off-by: Damien George <damien@micropython.org>
Diffstat (limited to 'tests')
-rw-r--r-- | tests/extmod/vfs_rom.py | 73 |
1 files changed, 73 insertions, 0 deletions
diff --git a/tests/extmod/vfs_rom.py b/tests/extmod/vfs_rom.py index f7958a9396..dc88481c02 100644 --- a/tests/extmod/vfs_rom.py +++ b/tests/extmod/vfs_rom.py @@ -223,6 +223,79 @@ class TestEdgeCases(unittest.TestCase): self.assertEqual(f.read(), b"contents") +class TestCorrupt(unittest.TestCase): + def test_corrupt_filesystem(self): + # Make the filesystem length bigger than the buffer. + romfs = bytearray(make_romfs(())) + romfs[3] = 0x01 + with self.assertRaises(OSError): + vfs.VfsRom(romfs) + + # Corrupt the filesystem length. + romfs = bytearray(make_romfs(())) + romfs[3] = 0xFF + with self.assertRaises(OSError): + vfs.VfsRom(romfs) + + # Corrupt the contents of the filesystem. + romfs = bytearray(make_romfs(())) + romfs[3] = 0x01 + romfs.extend(b"\xff\xff") + fs = vfs.VfsRom(romfs) + with self.assertRaises(OSError): + fs.stat("a") + self.assertEqual(list(fs.ilistdir("")), []) + + def test_corrupt_file_entry(self): + romfs = make_romfs((("file", b"data"),)) + + # Corrupt the length of filename. + romfs_corrupt = bytearray(romfs) + romfs_corrupt[7:] = b"\xff" * (len(romfs) - 7) + fs = vfs.VfsRom(romfs_corrupt) + with self.assertRaises(OSError): + fs.stat("file") + self.assertEqual(list(fs.ilistdir("")), []) + + # Erase the data record (change it to a padding record). + romfs_corrupt = bytearray(romfs) + romfs_corrupt[12] = VfsRomWriter.ROMFS_RECORD_KIND_PADDING + fs = vfs.VfsRom(romfs_corrupt) + with self.assertRaises(OSError): + fs.stat("file") + self.assertEqual(list(fs.ilistdir("")), []) + + # Corrupt the header of the data record. + romfs_corrupt = bytearray(romfs) + romfs_corrupt[12:] = b"\xff" * (len(romfs) - 12) + fs = vfs.VfsRom(romfs_corrupt) + with self.assertRaises(OSError): + fs.stat("file") + + # Corrupt the interior of the data record. + romfs_corrupt = bytearray(romfs) + romfs_corrupt[13:] = b"\xff" * (len(romfs) - 13) + fs = vfs.VfsRom(romfs_corrupt) + with self.assertRaises(OSError): + fs.stat("file") + + # Change the data record to an indirect pointer and corrupt the length. + romfs_corrupt = bytearray(romfs) + romfs_corrupt[12] = VfsRomWriter.ROMFS_RECORD_KIND_DATA_POINTER + romfs_corrupt[14:18] = b"\xff\xff\xff\xff" + fs = vfs.VfsRom(romfs_corrupt) + with self.assertRaises(OSError): + fs.stat("file") + + # Change the data record to an indirect pointer and corrupt the offset. + romfs_corrupt = bytearray(romfs) + romfs_corrupt[12] = VfsRomWriter.ROMFS_RECORD_KIND_DATA_POINTER + romfs_corrupt[14:18] = b"\x00\xff\xff\xff" + fs = vfs.VfsRom(romfs_corrupt) + with self.assertRaises(OSError): + fs.stat("file") + + class TestStandalone(TestBase): def test_constructor(self): self.assertIsInstance(vfs.VfsRom(self.romfs), vfs.VfsRom) |