Added and grouped generic attribute checks

Checked that attributes are [non-]resident when they have to be, and grouped consistency checks on each of them in a dedicated function. Consequenly request the checks where needed and remove existing index checks.
2021-07-12 08:31:18 +02:00 · 2021-07-12 08:31:18 +02:00 · 7f45544ed7
parent 436fe09f87
commit 7f45544ed7
6 changed files with 222 additions and 74 deletions
--- a/include/ntfs-3g/attrib.h
+++ b/include/ntfs-3g/attrib.h
@ -398,6 +398,7 @@ extern int ntfs_attr_data_write(ntfs_inode *ni,
 		const char *buf, size_t size, off_t offset);
 extern int ntfs_attr_shrink_size(ntfs_inode *ni, ntfschar *stream_name,
 		int stream_name_len, off_t offset);
+extern int ntfs_attr_consistent(const ATTR_RECORD *a, const MFT_REF mref);

 #endif /* defined _NTFS_ATTRIB_H */

--- a/libntfs-3g/attrib.c
+++ b/libntfs-3g/attrib.c
@ -3356,6 +3356,170 @@ not_found:
 	}
 }

+/*
+ *		Check the consistency of an attribute
+ *
+ *	Do the general consistency checks of the selected attribute :
+ *	- the required fields can be accessed
+ *	- the variable fields do not overflow
+ *	- the attribute is [non-]resident if it must be
+ *	- miscelleaneous checks
+ *
+ *	Returns 0 if the checks pass
+ *		-1 with errno = EIO otherwise
+ */
+
+int ntfs_attr_consistent(const ATTR_RECORD *a, const MFT_REF mref)
+{
+	const FILE_NAME_ATTR *fn;
+	const INDEX_ROOT *ir;
+	u64 inum;
+	int ret;
+
+	/*
+	 * The attribute was found to fully lie within the MFT
+	 * record, now make sure its relevant parts (name, runlist,
+	 * value) also lie within. The first step is to make sure
+	 * the attribute has the minimum length so that accesses to
+	 * the lengths and offsets of these parts are safe.
+	 */
+	ret = 0;
+	inum = MREF(mref);
+	if (a->non_resident) {
+		if ((a->non_resident != 1)
+		    || (le32_to_cpu(a->length)
+			< offsetof(ATTR_RECORD, non_resident_end))
+		    || (le16_to_cpu(a->mapping_pairs_offset)
+				>= le32_to_cpu(a->length))
+		    || (a->name_length
+			 && (((u32)le16_to_cpu(a->name_offset)
+				+ a->name_length * sizeof(ntfschar))
+				> le32_to_cpu(a->length)))
+		    || (le64_to_cpu(a->highest_vcn)
+				< le64_to_cpu(a->lowest_vcn))) {
+			ntfs_log_error("Corrupt non resident attribute"
+				" 0x%x in MFT record %lld\n",
+				(int)le32_to_cpu(a->type),
+				(long long)inum);
+			errno = EIO;
+			ret = -1;
+		}
+	} else {
+		if ((le32_to_cpu(a->length)
+			< offsetof(ATTR_RECORD, resident_end))
+		    || (le32_to_cpu(a->value_length) & 0xff000000)
+		    || (a->value_length
+			&& ((le16_to_cpu(a->value_offset)
+				+ le32_to_cpu(a->value_length))
+				> le32_to_cpu(a->length)))
+		    || (a->name_length
+			&& (((u32)le16_to_cpu(a->name_offset)
+				+ a->name_length * sizeof(ntfschar))
+				> le32_to_cpu(a->length)))) {
+			ntfs_log_error("Corrupt resident attribute"
+				" 0x%x in MFT record %lld\n",
+				(int)le32_to_cpu(a->type),
+				(long long)inum);
+			errno = EIO;
+			ret = -1;
+		}
+	}
+	if (!ret) {
+		/*
+		 * Checking whether an attribute must be [non-]resident
+		 * is hard-coded for well-known ones. This should be
+		 * done through ntfs_attr_can_be_non_resident(), based on
+		 * $AttrDef, but this would give an easy way to bypass
+		 * the checks.
+		 * Attributes which are not well-known are not checked.
+		 *
+		 * Note : at this stage we know that a->length and
+		 * a->value_length cannot look like being negative.
+		 */
+		switch(a->type) {
+		case AT_FILE_NAME :
+			/* Check file names are resident and do not overflow */
+			fn = (const FILE_NAME_ATTR*)((const u8*)a
+				+ le16_to_cpu(a->value_offset));
+			if (a->non_resident
+			    || (le32_to_cpu(a->value_length)
+				< offsetof(FILE_NAME_ATTR, file_name))
+			    || !fn->file_name_length
+			    || ((fn->file_name_length * sizeof(ntfschar)
+				+ offsetof(FILE_NAME_ATTR, file_name))
+				> le32_to_cpu(a->value_length))) {
+				ntfs_log_error("Corrupt file name"
+					" attribute in MFT record %lld.\n",
+					(long long)inum);
+				errno = EIO;
+				ret = -1;
+			}
+			break;
+		case AT_INDEX_ROOT :
+			/* Check root index is resident and does not overflow */
+			ir = (const INDEX_ROOT*)((const u8*)a +
+				le16_to_cpu(a->value_offset));
+			/* index.allocated_size may overflow while resizing */
+			if (a->non_resident
+			    || (le32_to_cpu(a->value_length)
+				< offsetof(INDEX_ROOT, index.reserved))
+			    || (le32_to_cpu(ir->index.index_length)
+					& 0xff000000)
+			    || ((le32_to_cpu(a->value_length)
+				- le32_to_cpu(ir->index.index_length))
+				< offsetof(INDEX_ROOT,index))
+			    || ((le32_to_cpu(a->value_length)
+				- le32_to_cpu(ir->index.index_length))
+				< le32_to_cpu(ir->index.entries_offset))
+			    || (le32_to_cpu(ir->index.index_length)
+				> le32_to_cpu(ir->index.allocated_size))) {
+				ntfs_log_error("Corrupt index root"
+					" in MFT record %lld.\n",
+					(long long)inum);
+				errno = EIO;
+				ret = -1;
+			}
+			break;
+		case AT_STANDARD_INFORMATION :
+		case AT_OBJECT_ID :
+		case AT_VOLUME_NAME :
+		case AT_EA_INFORMATION :
+			if (a->non_resident) {
+				ntfs_log_error("Attribute 0x%x in MFT record"
+					" %lld should be resident.\n",
+					(int)le32_to_cpu(a->type),
+					(long long)inum);
+				errno = EIO;
+				ret = -1;
+			}
+			break;
+		case AT_VOLUME_INFORMATION :
+			if (a->non_resident
+			    || (le32_to_cpu(a->value_length)
+					< sizeof(VOLUME_INFORMATION))) {
+				ntfs_log_error("Corrupt volume information"
+					" in MFT record %lld\n",
+					(long long)inum);
+				errno = EIO;
+				ret = -1;
+			}
+			break;
+		case AT_INDEX_ALLOCATION :
+			if (!a->non_resident) {
+				ntfs_log_error("Corrupt index allocation"
+					" in MFT record %lld",
+					(long long)inum);
+				errno = EIO;
+				ret = -1;
+			}
+			break;
+		default :
+			break;
+		}
+	}
+	return (ret);
+}
+
 /**
 * ntfs_attr_lookup - find an attribute in an ntfs inode
 * @type:	attribute type to find
@ -3430,7 +3594,6 @@ int ntfs_attr_lookup(const ATTR_TYPES type, const ntfschar *name,
 {
 	ntfs_volume *vol;
 	ntfs_inode *base_ni;
-	ATTR_RECORD *a;
 	int ret = -1;

 	ntfs_log_enter("Entering for attribute type 0x%x\n", le32_to_cpu(type));
@ -3452,50 +3615,6 @@ int ntfs_attr_lookup(const ATTR_TYPES type, const ntfschar *name,
 	else
 		ret = ntfs_external_attr_find(type, name, name_len, ic, 
 					      lowest_vcn, val, val_len, ctx);
-	if (!ret) {
-		/*
-		 * The attribute was found to fully lie within the MFT
-		 * record, now make sure its relevant parts (name, runlist,
-		 * value) also lie within. The first step is to make sure
-		 * the attribute has the minimum length so that accesses to
-		 * the lengths and offsets of these parts are safe.
-		 */
-		a = ctx->attr;
-		if (a->non_resident) {
-			if ((le32_to_cpu(a->length)
-				< offsetof(ATTR_RECORD, non_resident_end))
-			    || (le16_to_cpu(a->mapping_pairs_offset)
-					>= le32_to_cpu(a->length))
-			    || (a->name_length
-				 && ((le16_to_cpu(a->name_offset)
-					+ a->name_length)
-					> le32_to_cpu(a->length)))) {
-				ntfs_log_error("Corrupt non resident attribute"
-					" 0x%x in MFT record %lld\n",
-					(int)le32_to_cpu(a->type),
-					(long long)ctx->ntfs_ino->mft_no);
-				errno = EIO;
-				ret = -1;
-			}
-		} else {
-			if ((le32_to_cpu(a->length)
-				< offsetof(ATTR_RECORD, resident_end))
-			    || (le16_to_cpu(a->value_offset)
-					+ le32_to_cpu(a->value_length))
-					> le32_to_cpu(a->length)
-			    || (a->name_length
-				&& ((le16_to_cpu(a->name_offset)
-					+ a->name_length)
-					> le32_to_cpu(a->length)))) {
-				ntfs_log_error("Corrupt resident attribute 0x%x in"
-					" MFT record %lld\n",
-					(int)le32_to_cpu(a->type),
-					(long long)ctx->ntfs_ino->mft_no);
-				errno = EIO;
-				ret = -1;
-			}
-		}
-	}
 out:
 	ntfs_log_leave("\n");
 	return ret;
--- a/libntfs-3g/dir.c
+++ b/libntfs-3g/dir.c
@ -293,16 +293,7 @@ u64 ntfs_inode_lookup_by_name(ntfs_inode *dir_ni,
 				(unsigned)index_block_size);
 		goto put_err_out;
 	}
-	if (((offsetof(INDEX_ROOT,index)
-			+ le32_to_cpu(ir->index.allocated_size))
-			> le32_to_cpu(ctx->attr->value_length))
-	    || (le32_to_cpu(ir->index.entries_offset)
-			> le32_to_cpu(ir->index.index_length))
-	    || (le32_to_cpu(ir->index.index_length)
-			> le32_to_cpu(ir->index.allocated_size))) {
-		ntfs_log_error("Index root is corrupt.\n");
-		goto put_err_out;
-	}
+		/* Consistency check of ir done while fetching attribute */
 	index_end = (u8*)&ir->index + le32_to_cpu(ir->index.index_length);
 	/* The first index entry. */
 	ie = (INDEX_ENTRY*)((u8*)&ir->index +
@ -1097,12 +1088,6 @@ static MFT_REF ntfs_mft_get_parent_ref(ntfs_inode *ni)
 	}
 	fn = (FILE_NAME_ATTR*)((u8*)ctx->attr +
 			le16_to_cpu(ctx->attr->value_offset));
-	if ((u8*)fn +	le32_to_cpu(ctx->attr->value_length) >
-			(u8*)ctx->attr + le32_to_cpu(ctx->attr->length)) {
-		ntfs_log_error("Corrupt file name attribute in inode %lld.\n",
-			       (unsigned long long)ni->mft_no);
-		goto io_err_out;
-	}
 	mref = le64_to_cpu(fn->parent_directory);
 	ntfs_attr_put_search_ctx(ctx);
 	return mref;
--- a/libntfs-3g/index.c
+++ b/libntfs-3g/index.c
@ -730,7 +730,6 @@ int ntfs_index_lookup(const void *key, const int key_len, ntfs_index_context *ic
 	INDEX_ROOT *ir;
 	INDEX_ENTRY *ie;
 	INDEX_BLOCK *ib = NULL;
-	ATTR_RECORD *a;
 	int ret, err = 0;

 	ntfs_log_trace("Entering\n");
@ -771,17 +770,6 @@ int ntfs_index_lookup(const void *key, const int key_len, ntfs_index_context *ic
 	}
 	
 	old_vcn = VCN_INDEX_ROOT_PARENT;
-	a = icx->actx->attr;
-	if (((offsetof(INDEX_ROOT,index)
-			+ le32_to_cpu(ir->index.index_length))
-			> le32_to_cpu(a->value_length))
-	    || (le32_to_cpu(ir->index.entries_offset)
-			> le32_to_cpu(ir->index.index_length))) {
-		ntfs_log_error("Index root is corrupt in MFT record %lld.\n",
-				(long long)icx->ni->mft_no);
-		err = errno = ERANGE;
-		goto err_lookup;
-	}
 	ret = ntfs_ie_lookup(key, key_len, icx, &ir->index, &vcn, &ie);
 	if (ret == STATUS_ERROR) {
 		err = errno;
--- a/libntfs-3g/mft.c
+++ b/libntfs-3g/mft.c
@ -219,11 +219,26 @@ int ntfs_mft_records_write(const ntfs_volume *vol, const MFT_REF mref,
 	return -1;
 }

+/*
+ *		Check the consistency of an MFT record
+ *
+ *	Make sure its general fields are safe, then examine all its
+ *	attributes and apply generic checks to them.
+ *	The attribute checks are skipped when a record is being read in
+ *	order to collect its sequence number for creating a new record.
+ *
+ *	Returns 0 if the checks are successful
+ *		-1 with errno = EIO otherwise
+ */
+
 int ntfs_mft_record_check(const ntfs_volume *vol, const MFT_REF mref, 
 			  MFT_RECORD *m)
 {			  
 	ATTR_RECORD *a;
+	ATTR_TYPES previous_type;
 	int ret = -1;
+	u32 offset;
+	s32 space;
 	
 	if (!ntfs_is_file_record(m->magic)) {
 		if (!NVolNoFixupWarn(vol))
@ -240,7 +255,8 @@ int ntfs_mft_record_check(const ntfs_volume *vol, const MFT_REF mref,
 			       le32_to_cpu(m->bytes_allocated));
 		goto err_out;
 	}
-	if (le32_to_cpu(m->bytes_in_use) > vol->mft_record_size) {
+	if (!NVolNoFixupWarn(vol)
+	    && (le32_to_cpu(m->bytes_in_use) > vol->mft_record_size)) {
 		ntfs_log_error("Record %llu has corrupt in-use size "
 			       "(%u > %u)\n", (unsigned long long)MREF(mref),
 			       (int)le32_to_cpu(m->bytes_in_use),
@ -259,6 +275,37 @@ int ntfs_mft_record_check(const ntfs_volume *vol, const MFT_REF mref,
 			       (unsigned long long)MREF(mref));
 		goto err_out;
 	}
+
+	if (!NVolNoFixupWarn(vol)) {
+		offset = le16_to_cpu(m->attrs_offset);
+		space = le32_to_cpu(m->bytes_in_use) - offset;
+		a = (ATTR_RECORD*)((char*)m + offset);
+		previous_type = AT_STANDARD_INFORMATION;
+		while ((space >= (s32)offsetof(ATTR_RECORD, resident_end))
+		    && (a->type != AT_END)
+		    && (le32_to_cpu(a->type) >= le32_to_cpu(previous_type))) {
+			if ((le32_to_cpu(a->length) <= (u32)space)
+			    && !(le32_to_cpu(a->length) & 7)) {
+				if (!ntfs_attr_consistent(a, mref)) {
+					previous_type = a->type;
+					offset += le32_to_cpu(a->length);
+					space -= le32_to_cpu(a->length);
+					a = (ATTR_RECORD*)((char*)m + offset);
+				} else
+					goto err_out;
+			} else {
+				ntfs_log_error("Corrupted MFT record %llu\n",
+				       (unsigned long long)MREF(mref));
+				goto err_out;
+			}
+		}
+			/* We are supposed to reach an AT_END */
+		if ((space < 4) || (a->type != AT_END)) {
+			ntfs_log_error("Bad end of MFT record %llu\n",
+				       (unsigned long long)MREF(mref));
+			goto err_out;
+		}
+	}
 	
 	ret = 0;
 err_out:
--- a/libntfs-3g/volume.c
+++ b/libntfs-3g/volume.c
@ -976,6 +976,10 @@ ntfs_volume *ntfs_device_mount(struct ntfs_device *dev, ntfs_mount_flags flags)
 		}
 		goto error_exit;
 	}
+	for (i = 0; (i < l) && (i < FILE_first_user); ++i)
+		if (ntfs_mft_record_check(vol, FILE_MFT + i,
+				(MFT_RECORD*)(m + i*vol->mft_record_size)))
+			goto error_exit;
 	l = ntfs_attr_mst_pread(vol->mftmirr_na, 0, vol->mftmirr_size,
 			vol->mft_record_size, m2);
 	if (l != vol->mftmirr_size) {
@ -985,6 +989,10 @@ ntfs_volume *ntfs_device_mount(struct ntfs_device *dev, ntfs_mount_flags flags)
 		}
 		vol->mftmirr_size = l;
 	}
+	for (i = 0; (i < l) && (i < FILE_first_user); ++i)
+		if (ntfs_mft_record_check(vol, FILE_MFT + i,
+				(MFT_RECORD*)(m2 + i*vol->mft_record_size)))
+			goto error_exit;
 	ntfs_log_debug("Comparing $MFTMirr to $MFT...\n");
 		/* Windows 10 does not update the full $MFTMirr any more */
 	for (i = 0; (i < vol->mftmirr_size) && (i < FILE_first_user); ++i) {