Added better protection against internal leb128 underflow

There have already been a number of bugs that end up writing -1 out as
leb128s. The current encoder doesn't know the different betwee -1 and
0xffffffff, so asserting before this situation can happen is quite
important for preventing these bad leb128s from ever making it into a
stable version.

Also dropped LFS_ERR_OVERFLOW to use LFS_ERR_CORRUPT for bad leb128
encodings. These end up meaning the same thing to higher layers anyways.
This commit is contained in:
Christopher Haster
2023-04-04 12:12:49 -05:00
parent eb93c3b710
commit 355c7466f1
3 changed files with 15 additions and 7 deletions
+13 -3
View File
@@ -1311,6 +1311,10 @@ static lfs_ssize_t lfsr_rbyd_readtag(lfs_t *lfs,
}
delta += delta_;
if (id_ > 0x7fffffff) {
return LFS_ERR_CORRUPT;
}
lfs_size_t size_;
delta_ = lfs_fromleb128(&size_, &buffer[delta], 5);
if (delta_ < 0) {
@@ -1318,6 +1322,10 @@ static lfs_ssize_t lfsr_rbyd_readtag(lfs_t *lfs,
}
delta += delta_;
if (size_ > 0x7fffffff) {
return LFS_ERR_CORRUPT;
}
// optionally crc
if (crc) {
*crc = lfs_crc32c(*crc, buffer, delta);
@@ -1387,9 +1395,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
NULL, &lfs->rcache, limit-off,
block, off, &tag, &id, &size, &crc);
if (delta < 0) {
if (delta == LFS_ERR_INVAL
|| delta == LFS_ERR_CORRUPT
|| delta == LFS_ERR_OVERFLOW) {
if (delta == LFS_ERR_INVAL || delta == LFS_ERR_CORRUPT) {
maybeerased = maybeerased && delta == LFS_ERR_INVAL;
break;
}
@@ -1846,6 +1852,10 @@ static int lfsr_rbyd_progdata(lfs_t *lfs, lfsr_rbyd_t *rbyd_,
static int lfsr_rbyd_progtag(lfs_t *lfs, lfsr_rbyd_t *rbyd_,
lfsr_tag_t tag, lfs_ssize_t id, lfs_size_t size, uint32_t *crc) {
// check for underflow issues
LFS_ASSERT((lfs_size_t)(id+1) < 0x80000000);
LFS_ASSERT(size < 0x80000000);
// make sure to include the parity of the current crc
tag |= lfs_popc(rbyd_->crc) & 1;
-2
View File
@@ -88,8 +88,6 @@ enum lfs_error {
LFS_ERR_NOATTR = -61, // No data/attr available
LFS_ERR_NAMETOOLONG = -36, // File name too long
LFS_ERR_RANGE = -34, // Result out of range
// TODO should all overflow errors actually be corrupt errors?
LFS_ERR_OVERFLOW = -75, // Value too large for defined data type
};
// File types
+2 -2
View File
@@ -30,7 +30,7 @@ ssize_t lfs_toleb128(uint32_t word, void *buffer, size_t size) {
}
}
return LFS_ERR_OVERFLOW;
return LFS_ERR_CORRUPT;
}
ssize_t lfs_fromleb128(uint32_t *word, const void *buffer, size_t size) {
@@ -46,7 +46,7 @@ ssize_t lfs_fromleb128(uint32_t *word, const void *buffer, size_t size) {
}
}
return LFS_ERR_OVERFLOW;
return LFS_ERR_CORRUPT;
}