[PATCH 1/2] volume_id/ntfs: bound attribute and label reads to the MFT record

The NTFS prober walks the $Volume MFT record using offsets read
directly from the disk image. However, mftr->attrs_offset and each
attr->value_offset are used without checking that they fall within the
MFT record.

A crafted NTFS image can therefore cause the prober to read beyond the
record buffer:

- attrs_offset, or an attacker-controlled attr->len used while walking
  the attributes, can move the attribute header past the end of the
  record. The prober then reads the attribute's type, length, and
  value-related fields out of bounds.
- value_offset is added to the attribute start to locate the volume
  label, again without checking the resulting offset. The prober can
  consequently read up to VOLUME_ID_LABEL_SIZE bytes past the end of
  the record.

Both offsets come directly from the image and can be reached through
normal blkid/lsblk probing of a crafted filesystem image, such as one
on a malicious USB device automatically probed by udev.

When the out-of-bounds data is readable, those bytes are decoded as the
volume label and can expose adjacent heap memory. If the calculated
address falls into an unmapped page, the prober can crash.

The issue was confirmed with AddressSanitizer on a crafted image:

- READ of size 4 in volume_id_probe_ntfs while reading the attribute
  header
- READ of size 1 in volume_id_set_unicode16 while reading the volume
  label

The fix is to verify that the attribute header is fully contained
within the MFT record before accessing it, and to validate the
calculated label offset and length before reading the volume label.

              text    data     bss     dec     hex filename
   before      286       0       0     286     11e util-linux/volume_id/ntfs.o
   after       329       0       0     329     149 util-linux/volume_id/ntfs.o
                                       +43

Signed-off-by: Ali Ahmet Memis <[email protected]>
---
 util-linux/volume_id/ntfs.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/util-linux/volume_id/ntfs.c b/util-linux/volume_id/ntfs.c
index 22dd77fc7..19055e7db 100644
--- a/util-linux/volume_id/ntfs.c
+++ b/util-linux/volume_id/ntfs.c
@@ -154,19 +154,22 @@ int FAST_FUNC volume_id_probe_ntfs(struct volume_id *id /*,uint64_t off*/)
 	while (1) {
 		struct file_attribute *attr;
 
+		/* Attribute header (and the fields read below) must be
+		 * within the record. attr_off comes from the on-disk
+		 * mftr->attrs_offset and attr->len, both attacker-controlled.
+		 */
+		if (attr_off + sizeof(*attr) > mft_record_size)
+			break;
+
 		attr = (struct file_attribute*) &buf[attr_off];
 		attr_type = le32_to_cpu(attr->type);
 		attr_len = le32_to_cpu(attr->len);
 		val_off = le16_to_cpu(attr->value_offset);
 		val_len = le32_to_cpu(attr->value_len);
-		attr_off += attr_len;
 
 		if (attr_len == 0)
 			break;
 
-		if (attr_off >= mft_record_size)
-			break;
-
 		if (attr_type == MFT_RECORD_ATTR_END)
 			break;
 
@@ -186,10 +189,19 @@ int FAST_FUNC volume_id_probe_ntfs(struct volume_id *id /*,uint64_t off*/)
 			if (val_len > VOLUME_ID_LABEL_SIZE)
 				val_len = VOLUME_ID_LABEL_SIZE;
 
-			val = ((uint8_t *) attr) + val_off;
-//			volume_id_set_label_raw(id, val, val_len);
-			volume_id_set_label_unicode16(id, val, LE, val_len);
+			/* The value is at attr_off + val_off; make sure it
+			 * (and val_len bytes of it) lie within the record.
+			 */
+			if (val_off <= mft_record_size - attr_off
+			 && val_len <= mft_record_size - attr_off - val_off
+			) {
+				val = ((uint8_t *) attr) + val_off;
+//				volume_id_set_label_raw(id, val, val_len);
+				volume_id_set_label_unicode16(id, val, LE, val_len);
+			}
 		}
+
+		attr_off += attr_len;
 	}
 
  found:
-- 
2.55.0