set_fatent_value() and flush_dirty_fat_buffer() in fs/fat/fat_write.c
fill the FAT cache (mydata->fatbuf, FATBUFSIZE bytes) using a sector
count, getsize, derived from the on-disk fatlength:
if (startblock + getsize > fatlength)
getsize = fatlength - startblock;
getsize is a signed int and fatlength comes from the BPB without
validation, so when startblock > fatlength this underflows and the
wrapped value is passed to disk_read()/disk_write() as a __u32 block
count. In set_fatent_value() disk_read() has no bound of its own, so a
crafted FAT (tiny fatlength plus a directory entry with a large start
cluster, reached on the overwrite/truncate path) makes it read far
past the fixed-size fatbuf -- an out-of-bounds write of attacker disk
contents. flush_dirty_fat_buffer() has the identical computation
feeding disk_write().
Commit 87d85139a96a ("fs: fat: Perform sanity checks on getsize in
get_fatent()") added the same clamp to the read path in fs/fat/fat.c;
it did not touch fs/fat/fat_write.c. Make getsize unsigned and clamp
it to FATBUFBLOCKS at both write-path sites.
Fixes: 87d85139a96a ("fs: fat: Perform sanity checks on getsize in
get_fatent()")
Signed-off-by: Shubham Antil <[email protected]>
Cc: Tom Rini <[email protected]>
Cc: Heinrich Schuchardt <[email protected]>
Cc: Simon Glass <[email protected]>
Cc: Maxin B. John <[email protected]>
---
fs/fat/fat_write.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index a1d28b4e..02b2d313 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -219,7 +219,7 @@ static int disk_write(__u32 block, __u32 nr_blocks, void
*buf)
*/
static int flush_dirty_fat_buffer(fsdata *mydata)
{
- int getsize = FATBUFBLOCKS;
+ __u32 getsize = FATBUFBLOCKS;
__u32 fatlength = mydata->fatlength;
__u8 *bufptr = mydata->fatbuf;
__u32 startblock = mydata->fatbufnum * FATBUFBLOCKS;
@@ -236,6 +236,11 @@ static int flush_dirty_fat_buffer(fsdata *mydata)
startblock += mydata->fat_sect;
+ if (getsize > FATBUFBLOCKS) {
+ debug("getsize is too large for bufptr\n");
+ getsize = FATBUFBLOCKS;
+ }
+
/* Write FAT buf */
if (disk_write(startblock, getsize, bufptr) < 0) {
debug("error: writing FAT blocks\n");
@@ -465,7 +470,7 @@ static int set_fatent_value(fsdata *mydata, __u32 entry,
__u32 entry_value)
/* Read a new block of FAT entries into the cache. */
if (bufnum != mydata->fatbufnum) {
- int getsize = FATBUFBLOCKS;
+ __u32 getsize = FATBUFBLOCKS;
__u8 *bufptr = mydata->fatbuf;
__u32 fatlength = mydata->fatlength;
__u32 startblock = bufnum * FATBUFBLOCKS;
@@ -479,6 +484,11 @@ static int set_fatent_value(fsdata *mydata, __u32 entry,
__u32 entry_value)
startblock += mydata->fat_sect;
+ if (getsize > FATBUFBLOCKS) {
+ debug("getsize is too large for bufptr\n");
+ getsize = FATBUFBLOCKS;
+ }
+
if (disk_read(startblock, getsize, bufptr) < 0) {
debug("Error reading FAT blocks\n");
return -1;
--
2.43.0
