DF-3248 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 | # DF-3248 fix — bound the dirent walk in hpfs_genlookupbyname before any dereference. # Covers the read escape (DF-0927) and the write-through sink (DF-3248, hpfs_updateparent). # NOTE: hpfs_readdir (DF-0830) and hpfs_validateparent (DF-0865/DF-3143) replicate this # walk and need the same discipline — out of this diff's scope. --- a/sys/vfs/hpfs/hpfs_lookup.c +++ b/sys/vfs/hpfs/hpfs_lookup.c @@ -46,6 +46,55 @@ int hpfs_removedirent (struct hpfsmount *, lsn_t, char *, int, int *); +#define HPFS_MAXDIRDEPTH 64 /* b-tree dive depth cap */ + +/* + * A dirent read from an on-disk dirblk may only be trusted if it lies + * entirely within the block's busy area [sizeof(dirblk_t), d_freeoff) and + * its declared length is self-consistent. All of this is raw on-disk + * metadata on a crafted filesystem (DF-0927 read side; DF-3248 write side + * via hpfs_updateparent), so it must be validated before any dereference. + */ +static int +hpfs_dirent_sane (const struct dirblk *dp, const struct hpfsdirent *dep) +{ + caddr_t dend = (caddr_t)dp + dp->d_freeoff; + caddr_t d = (caddr_t)dep; + + if (d < (caddr_t)dp + sizeof(dirblk_t)) + return (0); + /* whole fixed part (incl. de_namelen/de_name[0]) inside busy area */ + if (d + sizeof(struct hpfsdirent) > dend) + return (0); + /* reclen must cover at least the fixed part plus the name */ + if (dep->de_reclen < + offsetof(struct hpfsdirent, de_name) + dep->de_namelen) + return (0); + /* the dirent this reclen advances to must stay in the busy area */ + if (dep->de_reclen > (size_t)(dend - d)) + return (0); + return (1); +} + +/* + * The END marker dirent is consumed outside the regular loop; before its + * DE_DOWNLSN is read at dep + reclen - 4, require the marker and the + * trailing lsn to lie inside the 2048-byte block. + */ +static int +hpfs_endd_sane (const struct dirblk *dp, const struct hpfsdirent *dep) +{ + caddr_t d = (caddr_t)dep; + + if (d < (caddr_t)dp + sizeof(dirblk_t)) + return (0); + if (dep->de_reclen < sizeof(struct hpfsdirent) + sizeof(lsn_t)) + return (0); + if (d + dep->de_reclen > (caddr_t)dp + D_BSIZE) + return (0); + return (1); +} + /* * This routine traverse the b+ tree representing directory * looking for file named 'name'. Returns buf struct and hpfsdirent @@ -66,20 +115,39 @@ struct hpfsdirent *dep; lsn_t lsn; int error, res; + int depth = 0; dprintf(("hpfs_genlookupbyname(0x%x, %s (%d)): \n", dhp->h_no, name, namelen)); lsn = ((alleaf_t *)dhp->h_fn.fn_abd)->al_lsn; dive: + if (++depth > HPFS_MAXDIRDEPTH) { + kprintf("hpfs_genlookupbyname: dive depth exceeded\n"); + return (EINVAL); + } error = hpfs_breaddirblk (hpmp, lsn, &bp); if (error) return (error); dp = (struct dirblk *) bp->b_data; + if (dp->d_freeoff < sizeof(dirblk_t) || dp->d_freeoff > D_BSIZE) { + kprintf("hpfs_genlookupbyname: INSANE d_freeoff %u\n", + dp->d_freeoff); + brelse (bp); + return (EINVAL); + } dep = D_DIRENT(dp); - while(!(dep->de_flag & DE_END)) { + while (1) { + if (!hpfs_dirent_sane (dp, dep)) { + kprintf("hpfs_genlookupbyname: INSANE dirent chain\n"); + brelse (bp); + return (EINVAL); + } + if (dep->de_flag & DE_END) + break; + dprintf(("no: 0x%x, size: %d, name: %2d:%.*s, flag: 0x%x\n", dep->de_fnode, dep->de_size, dep->de_namelen, dep->de_namelen, dep->de_name, dep->de_flag)); @@ -97,6 +165,11 @@ } if (dep->de_flag & DE_DOWN) { + if (!hpfs_endd_sane (dp, dep)) { + kprintf("hpfs_genlookupbyname: INSANE end dirent\n"); + brelse (bp); + return (EINVAL); + } lsn = DE_DOWNLSN(dep); brelse(bp); goto dive; |