Hi,

there should be some more validations in boot.c.  Not just to validate
the filesystem, but to make sure that the program won't crash later on
because it assumes some values...

First chunk:  The sectors per cluster have to be a power of 2, otherwise
it's possible to send fsck_msdos over the edge at for-loop comparison
with values that could go up to SIZE_MAX.  If it's not power of 2, it
can iterate above, overflow, and continue the loop.  By specifications
and our kernel implementation, the value has to be power of 2 anyway.

Second chunk is a merge from Android. A FAT filesystem without a FAT
doesn't make sense.

Third chunk:  A FAT without any sectors is invalid.  We would allocate
memory based on that, yet the memory will be accessed because the code
assumes that offsets in first sector is available.

Fourth and last chunk:  The offset into the partition must not be
larger than the filesystem.  If that's not checked, the previously
assigned value could overflow due to negative numbers.


Tobias


Index: boot.c
===================================================================
RCS file: /cvs/src/sbin/fsck_msdos/boot.c,v
retrieving revision 1.20
diff -u -p -r1.20 boot.c
--- boot.c      16 Jun 2014 18:33:33 -0000      1.20
+++ boot.c      16 Jun 2014 19:01:51 -0000
@@ -88,12 +88,16 @@ readboot(int dosfs, struct bootblock *bo
                goto fail;
        }
        boot->SecPerClust = block[13];
-       if (boot->SecPerClust == 0) {
+       if (boot->SecPerClust == 0 || !powerof2(boot->SecPerClust)) {
                pfatal("Invalid cluster size: %u\n", boot->SecPerClust);
                goto fail;
        }
        boot->ResSectors = block[14] + (block[15] << 8);
        boot->FATs = block[16];
+       if (boot->FATs == 0) {
+               pfatal("Invalid number of FATs: %u\n", boot->FATs);
+               goto fail;
+       }
        boot->RootDirEnts = block[17] + (block[18] << 8);
        boot->Sectors = block[19] + (block[20] << 8);
        boot->Media = block[21];
@@ -227,6 +231,11 @@ readboot(int dosfs, struct bootblock *bo
                /* Check backup FSInfo?                                 XXX */
        }
 
+       if (boot->FATsecs == 0) {
+               pfatal("Invalid number of FAT sectors: %u\n", boot->FATsecs);
+               goto fail;
+       }
+
        boot->ClusterOffset = (boot->RootDirEnts * 32 + secsize - 1)
            / secsize
            + boot->ResSectors
@@ -239,6 +248,12 @@ readboot(int dosfs, struct bootblock *bo
        } else
                boot->NumSectors = boot->HugeSectors;
        boot->NumClusters = (boot->NumSectors - boot->ClusterOffset) / 
boot->SecPerClust;
+
+       if (boot->ClusterOffset > boot->NumSectors) {
+               pfatal("Cluster offset too large (%u clusters)\n",
+                   boot->ClusterOffset);
+               goto fail;
+       }
 
        if (boot->flags&FAT32)
                boot->ClustMask = CLUST32_MASK;

Reply via email to