diff --git a/sys/vfs/hpfs/hpfs.h b/sys/vfs/hpfs/hpfs.h --- a/sys/vfs/hpfs/hpfs.h +++ b/sys/vfs/hpfs/hpfs.h @@ -141,6 +141,19 @@ lsn_t d_parent; lsn_t d_self; } dirblk_t; +/* + * DF-0830: Bounds-check a dep pointer against its parent bread buffer. + * The dep-walk loops in hpfs_readdir / hpfs_validateparent / + * hpfs_genlookupbyname advance dep by de_reclen (attacker-controlled u16 + * off disk) with no bound check, so a crafted dir block walks dep past the + * 2 KB (D_BSIZE) buffer. This macro must be evaluated BEFORE any field of + * dep is read so an OOB dep never gets dereferenced. + */ +#define HPFS_DE_INBOUNDS(bp, dep) \ + ((caddr_t)(dep) >= (caddr_t)(bp)->b_data && \ + (caddr_t)(dep) + sizeof(hpfsdirent_t) <= \ + (caddr_t)(bp)->b_data + D_BSIZE) + /* * Allocation Block (ALBLK) diff --git a/sys/vfs/hpfs/hpfs_vnops.c b/sys/vfs/hpfs/hpfs_vnops.c --- a/sys/vfs/hpfs/hpfs_vnops.c +++ b/sys/vfs/hpfs/hpfs_vnops.c @@ -841,13 +841,21 @@ if (olsn) { dprintf(("[restore 0x%x] ", olsn)); - while(!(dep->de_flag & DE_END) ) { + while (HPFS_DE_INBOUNDS(bp, dep) && + dep->de_reclen >= sizeof(hpfsdirent_t) && + !(dep->de_flag & DE_END) ) { if((dep->de_flag & DE_DOWN) && (olsn == DE_DOWNLSN(dep))) break; dep = (hpfsdirent_t *)((caddr_t)dep + dep->de_reclen); } + if (!HPFS_DE_INBOUNDS(bp, dep)) { + kprintf("hpfs_readdir: dep out of bounds\n"); + brelse(bp); + error = EINVAL; + goto done; + } if((dep->de_flag & DE_DOWN) && (olsn == DE_DOWNLSN(dep))) { if (dep->de_flag & DE_END) goto blockdone; @@ -879,7 +887,9 @@ olsn = 0; - while(!(dep->de_flag & DE_END)) { + while (HPFS_DE_INBOUNDS(bp, dep) && + dep->de_reclen >= sizeof(hpfsdirent_t) && + !(dep->de_flag & DE_END)) { if(dep->de_flag & DE_DOWN) { lsn = DE_DOWNLSN(dep); brelse(bp); @@ -906,6 +916,8 @@ dep = (hpfsdirent_t *)((caddr_t)dep + dep->de_reclen); } + if (!HPFS_DE_INBOUNDS(bp, dep)) + goto blockdone; if(dep->de_flag & DE_DOWN) { dprintf(("[enddive] ")); lsn = DE_DOWNLSN(dep); diff --git a/sys/vfs/hpfs/hpfs_subr.c b/sys/vfs/hpfs/hpfs_subr.c --- a/sys/vfs/hpfs/hpfs_subr.c +++ b/sys/vfs/hpfs/hpfs_subr.c @@ -569,13 +569,20 @@ if (olsn) { dprintf(("[restore 0x%x] ", olsn)); - while(!(dep->de_flag & DE_END) ) { + while (HPFS_DE_INBOUNDS(bp, dep) && + dep->de_reclen >= sizeof(hpfsdirent_t) && + !(dep->de_flag & DE_END) ) { if((dep->de_flag & DE_DOWN) && (olsn == DE_DOWNLSN(dep))) break; dep = (hpfsdirent_t *)((caddr_t)dep + dep->de_reclen); } + if (!HPFS_DE_INBOUNDS(bp, dep)) { + kprintf("hpfs_validateparent: dep out of bounds\n"); + error = EINVAL; + goto failed; + } if((dep->de_flag & DE_DOWN) && (olsn == DE_DOWNLSN(dep))) { if (dep->de_flag & DE_END) goto blockdone; @@ -595,7 +602,9 @@ olsn = 0; - while(!(dep->de_flag & DE_END)) { + while (HPFS_DE_INBOUNDS(bp, dep) && + dep->de_reclen >= sizeof(hpfsdirent_t) && + !(dep->de_flag & DE_END)) { if(dep->de_flag & DE_DOWN) { lsn = DE_DOWNLSN(dep); level++; @@ -610,6 +619,8 @@ dep = (hpfsdirent_t *)((caddr_t)dep + dep->de_reclen); } + if (!HPFS_DE_INBOUNDS(bp, dep)) + goto failed; if(dep->de_flag & DE_DOWN) { dprintf(("[enddive] ")); lsn = DE_DOWNLSN(dep); diff --git a/sys/vfs/hpfs/hpfs_lookup.c b/sys/vfs/hpfs/hpfs_lookup.c --- a/sys/vfs/hpfs/hpfs_lookup.c +++ b/sys/vfs/hpfs/hpfs_lookup.c @@ -79,7 +79,9 @@ dp = (struct dirblk *) bp->b_data; dep = D_DIRENT(dp); - while(!(dep->de_flag & DE_END)) { + while (HPFS_DE_INBOUNDS(bp, dep) && + dep->de_reclen >= sizeof(hpfsdirent_t) && + !(dep->de_flag & DE_END)) { 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)); @@ -96,6 +98,10 @@ dep = (hpfsdirent_t *)(((caddr_t)dep) + dep->de_reclen); } + if (!HPFS_DE_INBOUNDS(bp, dep)) { + brelse(bp); + return (EINVAL); + } if (dep->de_flag & DE_DOWN) { lsn = DE_DOWNLSN(dep); brelse(bp);