From 3f4984d33fea2916562c39e300d7ff683d4cbe9e Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 29 Jan 2025 15:10:08 -0600 Subject: [PATCH] Fixed truncated cksum tags reading past end-of-block While we do check for out-of-bound tags in lfsr_bd_readtag, we were ignoring the returned size in lfsr_rbyd_fetch when reading cksum tags. This meant it was possible for lfsr_rbyd_fetch to try to read past the end-of-block if: 1. The cksum tag was malformed with size < 4. 2. The malformed cksum tag was < 4 bytes from the end-of-block. A pretty rare case! Considering we don't even bother writing cksum tags when we're that close to the end-of-block. This can only happen in our tests if existing garbage happens to look like a cksum tag. While every cksum tag _should_ have at least 4 bytes for the cksum, we can't guarantee that if we're parsing garbage. Found by our test_ck_spam_dir_fuzz test. --- I've also added a couple test_mtree_truncated_* tests to catch similar truncation issues and prevent a regression in the future. We can't really rely on test_ck_spam_* to always find nuanced errors like this, but it's neat it found this one. Code changes: code stack ctx before: 38340 2624 640 after: 38344 (+0.0%) 2624 (+0.0%) 640 (+0.0%) --- lfs.c | 5 + tests/test_mtree.toml | 242 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+) diff --git a/lfs.c b/lfs.c index 65a56e21..13e4e429 100644 --- a/lfs.c +++ b/lfs.c @@ -2828,6 +2828,11 @@ static int lfsr_rbyd_fetch_(lfs_t *lfs, // is an end-of-commit cksum } else { + // truncate checksum? + if (size < sizeof(uint32_t)) { + break; + } + // check checksum uint32_t cksum__ = 0; err = lfsr_bd_read(lfs, block, off_, -1, diff --git a/tests/test_mtree.toml b/tests/test_mtree.toml index 7162b807..13f4fa35 100644 --- a/tests/test_mtree.toml +++ b/tests/test_mtree.toml @@ -4390,6 +4390,248 @@ code = ''' ''' +## Truncate mroot tests ## + +# test some that some tricky truncated tags are rejected correctly +[cases.test_mtree_truncated_tag] +defines.OVERFLOW = [-3, -2, -1, 0, 1, 2, 3, 4, 5] +in = 'lfs.c' +code = ''' + // create a malformed mroot + uint8_t buffer[BLOCK_SIZE]; + // fill with zeros to make parity checks easier + memset(buffer, 0, BLOCK_SIZE); + memcpy(&buffer[0], "evil", 4); + uint32_t cksum = lfs_crc32c(0, &buffer[0], 4); + + // make sure we're not caught by magic checks + buffer[4+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_MAGIC >> 8); + buffer[4+1] = (uint8_t)(LFSR_TAG_MAGIC >> 0); + buffer[4+2] = 0; + buffer[4+3] = 8; + memcpy(&buffer[4+4], "littlefs", 8); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[4], 4+8); + + // append a tag that overflows our block + lfs_size_t size = BLOCK_SIZE - (16+7) + OVERFLOW; + buffer[16+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_ATTR >> 8); + buffer[16+1] = (uint8_t)(LFSR_TAG_ATTR >> 0); + buffer[16+2] = 0; + buffer[16+3] = 0x80 | (0x7f & (size >> 0)); + buffer[16+4] = 0x80 | (0x7f & (size >> 7)); + buffer[16+5] = 0x80 | (0x7f & (size >> 14)); + buffer[16+6] = 0x00 | (0x7f & (size >> 21)); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[16], 7+size); + + // make next tag look valid to make errors look more likely + if (OVERFLOW < 0) { + buffer[BLOCK_SIZE + OVERFLOW] = ((uint8_t)lfs_parity(cksum) << 7); + } + + // write to both mroot blocks + for (int i = 0; i < 2; i++) { + CFG->erase(CFG, i) => 0; + CFG->prog(CFG, i, 0, buffer, BLOCK_SIZE) => 0; + } + + // try to mount, this should fail + lfs_t lfs; + lfsr_mount(&lfs, LFS_M_RDWR | M_FLAGS, CFG) => LFS_ERR_CORRUPT; +''' + +[cases.test_mtree_truncated_cksum] +defines.OVERFLOW = [1, 2, 3, 4] +defines.TRUNCATED_SIZE = [false, true] +in = 'lfs.c' +code = ''' + // create a malformed mroot + uint8_t buffer[BLOCK_SIZE]; + // fill with zeros to make parity checks easier + memset(buffer, 0, BLOCK_SIZE); + memcpy(&buffer[0], "evil", 4); + uint32_t cksum = lfs_crc32c(0, &buffer[0], 4); + + // make sure we're not caught by magic checks + buffer[4+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_MAGIC >> 8); + buffer[4+1] = (uint8_t)(LFSR_TAG_MAGIC >> 0); + buffer[4+2] = 0; + buffer[4+3] = 8; + memcpy(&buffer[4+4], "littlefs", 8); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[4], 4+8); + + // append a tag for padding + lfs_size_t size = BLOCK_SIZE - (16+7) - (7+4) + OVERFLOW; + buffer[16+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_ATTR >> 8); + buffer[16+1] = (uint8_t)(LFSR_TAG_ATTR >> 0); + buffer[16+2] = 0; + buffer[16+3] = 0x80 | (0x7f & (size >> 0)); + buffer[16+4] = 0x80 | (0x7f & (size >> 7)); + buffer[16+5] = 0x80 | (0x7f & (size >> 14)); + buffer[16+6] = 0x00 | (0x7f & (size >> 21)); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[16], 7+size); + + // append a truncated cksum tag + lfs_off_t off = BLOCK_SIZE - (7+4) + OVERFLOW; + size = (TRUNCATED_SIZE) ? 4-OVERFLOW : 4; + buffer[off+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_CKSUM >> 8); + buffer[off+1] = (uint8_t)(LFSR_TAG_CKSUM >> 0); + buffer[off+2] = 0; + buffer[off+3] = 0x80 | (0x7f & (size >> 0)); + buffer[off+4] = 0x80 | (0x7f & (size >> 7)); + buffer[off+5] = 0x80 | (0x7f & (size >> 14)); + buffer[off+6] = 0x00 | (0x7f & (size >> 21)); + + // write to both mroot blocks + for (int i = 0; i < 2; i++) { + CFG->erase(CFG, i) => 0; + CFG->prog(CFG, i, 0, buffer, BLOCK_SIZE) => 0; + } + + // try to mount, this should fail + lfs_t lfs; + lfsr_mount(&lfs, LFS_M_RDWR | M_FLAGS, CFG) => LFS_ERR_CORRUPT; +''' + +[cases.test_mtree_truncated_ecksum] +defines.OVERFLOW = [-3, -2, -1, 0, 1, 2, 3, 4, 5] +defines.TRUNCATED_SIZE = [false, true] +in = 'lfs.c' +code = ''' + // create a malformed mroot + uint8_t buffer[BLOCK_SIZE]; + // fill with zeros to make parity checks easier + memset(buffer, 0, BLOCK_SIZE); + memcpy(&buffer[0], "evil", 4); + uint32_t cksum = lfs_crc32c(0, &buffer[0], 4); + + // make sure we're not caught by magic checks + buffer[4+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_MAGIC >> 8); + buffer[4+1] = (uint8_t)(LFSR_TAG_MAGIC >> 0); + buffer[4+2] = 0; + buffer[4+3] = 8; + memcpy(&buffer[4+4], "littlefs", 8); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[4], 4+8); + + // append a tag for padding + lfs_size_t size = BLOCK_SIZE - (16+7) - (7+5) + OVERFLOW; + buffer[16+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_ATTR >> 8); + buffer[16+1] = (uint8_t)(LFSR_TAG_ATTR >> 0); + buffer[16+2] = 0; + buffer[16+3] = 0x80 | (0x7f & (size >> 0)); + buffer[16+4] = 0x80 | (0x7f & (size >> 7)); + buffer[16+5] = 0x80 | (0x7f & (size >> 14)); + buffer[16+6] = 0x00 | (0x7f & (size >> 21)); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[16], 7+size); + + // append a truncated ecksum tag + lfs_off_t off = BLOCK_SIZE - (7+5) + OVERFLOW; + size = (TRUNCATED_SIZE) ? 5-lfs_smax(OVERFLOW, 0) : 5; + buffer[off+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_ECKSUM >> 8); + buffer[off+1] = (uint8_t)(LFSR_TAG_ECKSUM >> 0); + buffer[off+2] = 0; + buffer[off+3] = 0x80 | (0x7f & (size >> 0)); + buffer[off+4] = 0x80 | (0x7f & (size >> 7)); + buffer[off+5] = 0x80 | (0x7f & (size >> 14)); + buffer[off+6] = 0x00 | (0x7f & (size >> 21)); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[16], 7+size); + + // make next tag look valid to make errors look more likely + if (OVERFLOW < 0) { + buffer[BLOCK_SIZE + OVERFLOW] = ((uint8_t)lfs_parity(cksum) << 7); + } + + // write to both mroot blocks + for (int i = 0; i < 2; i++) { + CFG->erase(CFG, i) => 0; + CFG->prog(CFG, i, 0, buffer, BLOCK_SIZE) => 0; + } + + // try to mount, this should fail + lfs_t lfs; + lfsr_mount(&lfs, LFS_M_RDWR | M_FLAGS, CFG) => LFS_ERR_CORRUPT; +''' + +[cases.test_mtree_truncated_gcksumdelta] +defines.OVERFLOW = [-3, -2, -1, 0, 1, 2, 3, 4] +defines.TRUNCATED_SIZE = [false, true] +in = 'lfs.c' +code = ''' + // create a malformed mroot + uint8_t buffer[BLOCK_SIZE]; + // fill with zeros to make parity checks easier + memset(buffer, 0, BLOCK_SIZE); + memcpy(&buffer[0], "evil", 4); + uint32_t cksum = lfs_crc32c(0, &buffer[0], 4); + + // make sure we're not caught by magic checks + buffer[4+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_MAGIC >> 8); + buffer[4+1] = (uint8_t)(LFSR_TAG_MAGIC >> 0); + buffer[4+2] = 0; + buffer[4+3] = 8; + memcpy(&buffer[4+4], "littlefs", 8); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[4], 4+8); + + // append a tag for padding + lfs_size_t size = BLOCK_SIZE - (16+7) - (7+4) + OVERFLOW; + buffer[16+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_ATTR >> 8); + buffer[16+1] = (uint8_t)(LFSR_TAG_ATTR >> 0); + buffer[16+2] = 0; + buffer[16+3] = 0x80 | (0x7f & (size >> 0)); + buffer[16+4] = 0x80 | (0x7f & (size >> 7)); + buffer[16+5] = 0x80 | (0x7f & (size >> 14)); + buffer[16+6] = 0x00 | (0x7f & (size >> 21)); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[16], 7+size); + + // append a truncated gcksumdelta tag + lfs_off_t off = BLOCK_SIZE - (7+4) + OVERFLOW; + size = (TRUNCATED_SIZE) ? 4-lfs_smax(OVERFLOW, 0) : 4; + buffer[off+0] = ((uint8_t)lfs_parity(cksum) << 7) + | (uint8_t)(LFSR_TAG_GCKSUMDELTA >> 8); + buffer[off+1] = (uint8_t)(LFSR_TAG_GCKSUMDELTA >> 0); + buffer[off+2] = 0; + buffer[off+3] = 0x80 | (0x7f & (size >> 0)); + buffer[off+4] = 0x80 | (0x7f & (size >> 7)); + buffer[off+5] = 0x80 | (0x7f & (size >> 14)); + buffer[off+6] = 0x00 | (0x7f & (size >> 21)); + cksum = lfs_crc32c(cksum ^ ((uint32_t)lfs_parity(cksum) << 7), + &buffer[16], 7+size); + + // make next tag look valid to make errors look more likely + if (OVERFLOW < 0) { + buffer[BLOCK_SIZE + OVERFLOW] = ((uint8_t)lfs_parity(cksum) << 7); + } + + // write to both mroot blocks + for (int i = 0; i < 2; i++) { + CFG->erase(CFG, i) => 0; + CFG->prog(CFG, i, 0, buffer, BLOCK_SIZE) => 0; + } + + // try to mount, this should fail + lfs_t lfs; + lfsr_mount(&lfs, LFS_M_RDWR | M_FLAGS, CFG) => LFS_ERR_CORRUPT; +''' + + ## Magic consistency ## # make sure our magic string ("littlefs") shows up in the same place (off=8)