DF-0830 / 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 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 | 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); |