[PATCH 0/4] e2fsprogs: blkid: misc fixes

[PATCH 0/4] e2fsprogs: blkid: misc fixes

[Eric Sandeen]

This is a series of LLM-scan-inspired fixes to the built-in blkid in
e2fsprogs. This has been lightly tested against trivial ntfs and exfat
images with labels, to ensure they are still properly read.

[PATCH 1/4] blkid: probe_ntfs: fix attr->len endian swap size

[Eric Sandeen]

From: Eric Sandeen <esandeen@redhat.com>

attr->len is a __u32, so use blkid_le32(), and change attr_len (to which
it is assigned) to an unsigned int as well.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 lib/blkid/probe.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/blkid/probe.c b/lib/blkid/probe.c
index 6a3bb247..31d048db 100644
--- a/lib/blkid/probe.c
+++ b/lib/blkid/probe.c
@@ -689,8 +689,8 @@ static int probe_ntfs(struct blkid_probe *probe,
 	struct file_attribute *attr;
 	char		uuid_str[17], label_str[129], *cp;
 	int		bytes_per_sector, sectors_per_cluster;
-	int		mft_record_size, attr_off, attr_len;
-	unsigned int	i, attr_type, val_len;
+	int		mft_record_size, attr_off;
+	unsigned int	i, attr_len, attr_type, val_len;
 	int		val_off;
 	__u64		nr_clusters;
 	blkid_loff_t off;
@@ -752,7 +752,7 @@ static int probe_ntfs(struct blkid_probe *probe,
 
 	while (1) {
 		attr = (struct file_attribute *) (buf_mft + attr_off);
-		attr_len = blkid_le16(attr->len);
+		attr_len = blkid_le32(attr->len);
 		attr_type = blkid_le32(attr->type);
 		val_off = blkid_le16(attr->value_offset);
 		val_len = blkid_le32(attr->value_len);
-- 
2.55.0

[PATCH 2/4] blkid: probe_ntfs: validate on-disk attribute size/length

[Eric Sandeen]

From: Eric Sandeen <esandeen@redhat.com>

Ensure that the attribute length itself is valid, and that it does not
extend past the mft_record_size, which would result in OOB reads past
the mft buffer.

For tidiness, move attr_off advance to end of the loop.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 lib/blkid/probe.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/blkid/probe.c b/lib/blkid/probe.c
index 31d048db..ae769a77 100644
--- a/lib/blkid/probe.c
+++ b/lib/blkid/probe.c
@@ -751,16 +751,22 @@ static int probe_ntfs(struct blkid_probe *probe,
 	label_str[0] = 0;
 
 	while (1) {
+		/* Don't read past mft_record_size buf on invalid offset */
+		if (attr_off > mft_record_size || sizeof(*attr) > mft_record_size - attr_off)
+			break;
+
 		attr = (struct file_attribute *) (buf_mft + attr_off);
 		attr_len = blkid_le32(attr->len);
 		attr_type = blkid_le32(attr->type);
 		val_off = blkid_le16(attr->value_offset);
 		val_len = blkid_le32(attr->value_len);
 
-		attr_off += attr_len;
+		/* Attribute length must be at least as large as header */
+		if (attr_len < sizeof(*attr))
+			break;
 
-		if ((attr_off > mft_record_size) ||
-		    (attr_len == 0))
+		/* End of this attribute must be within total record size */
+		if (attr_len > mft_record_size - attr_off)
 			break;
 
 		if (attr_type == MFT_RECORD_ATTR_END)
@@ -778,6 +784,8 @@ static int probe_ntfs(struct blkid_probe *probe,
 			}
 			*cp = 0;
 		}
+
+		attr_off += attr_len;
 	}
 
 	sprintf(uuid_str, "%016llX", blkid_le64(ns->volume_serial));
-- 
2.55.0

[PATCH 3/4] blkid: probe_ntfs: validate offset and length of volume name

[Eric Sandeen]

From: Eric Sandeen <esandeen@redhat.com>

Ensure that the volume name fits within the attribute offset

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 lib/blkid/probe.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/blkid/probe.c b/lib/blkid/probe.c
index ae769a77..87167171 100644
--- a/lib/blkid/probe.c
+++ b/lib/blkid/probe.c
@@ -773,6 +773,13 @@ static int probe_ntfs(struct blkid_probe *probe,
 			break;
 
 		if (attr_type == MFT_RECORD_ATTR_VOLUME_NAME) {
+			/* Value length must be multiple of two (UTF-16LE) */
+			if (val_len & 1)
+				break;
+			/* End of value must be within attr length */
+			if (val_off > attr_len || val_len > attr_len - val_off)
+				break;
+
 			if (val_len > sizeof(label_str))
 				val_len = sizeof(label_str)-1;
 
-- 
2.55.0

[PATCH 4/4] blkid: probe_exfat: clamp untrusted label->length to valid size

[Eric Sandeen]

From: Eric Sandeen <esandeen@redhat.com>

A corrupted or invalid label->length in an exfat filesystem may cause
probing to read past the end of label->name. Limit it in the same manner
as util-linux uses, and add a min() macro here to match.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 lib/blkid/probe.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/blkid/probe.c b/lib/blkid/probe.c
index 87167171..c371acea 100644
--- a/lib/blkid/probe.c
+++ b/lib/blkid/probe.c
@@ -38,6 +38,8 @@
 #include "uuid/uuid.h"
 #include "probe.h"
 
+#define min(a, b) ((a) < (b) ? (a) : (b))
+
 static int figure_label_len(const unsigned char *label, int len)
 {
 	const unsigned char *end = label + len - 1;
@@ -1535,7 +1537,8 @@ static int probe_exfat(struct blkid_probe *probe,
     label = find_exfat_entry_label(probe, sb);
     if (label) {
         unsigned char utf8_label[128];
-        unicode_16le_to_utf8(utf8_label, sizeof(utf8_label), label->name, label->length * 2);
+        unicode_16le_to_utf8(utf8_label, sizeof(utf8_label), label->name,
+		min((size_t) label->length * 2, sizeof(label->name)));
         blkid_set_tag(probe->dev, "LABEL", (char *) utf8_label, 0);
     } else {
         blkid_set_tag(probe->dev, "LABEL", "disk", 4);
-- 
2.55.0