Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 28 Oct 2024 15:14:44 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 878ede1a0d0f - main - fstyp: Fix some memory safety bugs
Message-ID:  <202410281514.49SFEial053646@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=878ede1a0d0f10f851b2bc54be1e28f512bfc016

commit 878ede1a0d0f10f851b2bc54be1e28f512bfc016
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-10-28 13:51:58 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-10-28 15:03:53 +0000

    fstyp: Fix some memory safety bugs
    
    In the hammer2 label reader, make sure to check for a NULL return from
    read_buf().
    
    In the NTFS label reader,
    - Avoid an infinite loop if a record length is 0.
    - Avoid walking past the end of the buffer.
    - When a label is found, avoid reading past the end of the buffer.
    
    PR:             278281
    Reviewed by:    emaste
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D47292
---
 usr.sbin/fstyp/hammer2.c |  2 ++
 usr.sbin/fstyp/ntfs.c    | 36 ++++++++++++++++++++++++------------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/usr.sbin/fstyp/hammer2.c b/usr.sbin/fstyp/hammer2.c
index e0c036c7442a..aeb29762fb6b 100644
--- a/usr.sbin/fstyp/hammer2.c
+++ b/usr.sbin/fstyp/hammer2.c
@@ -220,6 +220,8 @@ read_label(FILE *fp, char *label, size_t size)
 		broot.data_off = (i * HAMMER2_ZONE_BYTES64) | HAMMER2_PBUFRADIX;
 		vols[i] = read_buf(fp, broot.data_off & ~HAMMER2_OFF_MASK_RADIX,
 		    sizeof(*vols[i]));
+		if (vols[i] == NULL)
+			errx(1, "failed to read volume header");
 		broot.mirror_tid = vols[i]->voldata.mirror_tid;
 		if (best_i < 0 || best.mirror_tid < broot.mirror_tid) {
 			best_i = i;
diff --git a/usr.sbin/fstyp/ntfs.c b/usr.sbin/fstyp/ntfs.c
index a3457c285edc..be8095d5ee27 100644
--- a/usr.sbin/fstyp/ntfs.c
+++ b/usr.sbin/fstyp/ntfs.c
@@ -137,9 +137,8 @@ fstyp_ntfs(FILE *fp, char *label, size_t size)
 	struct ntfs_filerec *fr;
 	struct ntfs_attr *atr;
 	off_t voloff;
-	char *ap;
 	int8_t mftrecsz;
-	int recsize;
+	size_t recsize;
 #endif /* WITH_ICONV */
 
 	filerecp = NULL;
@@ -152,7 +151,8 @@ fstyp_ntfs(FILE *fp, char *label, size_t size)
 		goto ok;
 
 	mftrecsz = bf->bf_mftrecsz;
-	recsize = (mftrecsz > 0) ? (mftrecsz * bf->bf_bps * bf->bf_spc) : (1 << -mftrecsz);
+	recsize = (mftrecsz > 0) ?
+	    (mftrecsz * bf->bf_bps * bf->bf_spc) : (1 << -mftrecsz);
 
 	voloff = bf->bf_mftcn * bf->bf_spc * bf->bf_bps +
 	    recsize * NTFS_VOLUMEINO;
@@ -165,16 +165,28 @@ fstyp_ntfs(FILE *fp, char *label, size_t size)
 	if (fr->fr_hdrmagic != NTFS_FILEMAGIC)
 		goto fail;
 
-	for (ap = filerecp + fr->fr_attroff;
-	    atr = (struct ntfs_attr *)ap, (int)atr->a_type != -1;
-	    ap += atr->reclen) {
-		if (atr->a_type != NTFS_A_VOLUMENAME)
-			continue;
-
-		convert_label(ap + atr->a_dataoff,
-		    atr->a_datalen, label, size);
-		break;
+	for (size_t ioff = fr->fr_attroff;
+	    ioff + sizeof(struct ntfs_attr) < recsize;
+	    ioff += atr->reclen) {
+		atr = (struct ntfs_attr *)(filerecp + ioff);
+		if ((int)atr->a_type == -1)
+			goto ok;
+		if (atr->a_type == NTFS_A_VOLUMENAME) {
+			if ((size_t)atr->a_dataoff + atr->a_datalen > recsize) {
+				warnx("ntfs: Volume name attribute overflow");
+				goto fail;
+			}
+			convert_label(filerecp + ioff + atr->a_dataoff,
+			    atr->a_datalen, label, size);
+			goto ok;
+		}
+		if (atr->reclen == 0) {
+			warnx("ntfs: Invalid attribute record length");
+			goto fail;
+		}
 	}
+	warnx("ntfs: Volume name not found");
+	goto fail;
 
 ok:
 #else



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202410281514.49SFEial053646>