[PATCH] hpfs: validate external EA record lengths

Samuel Moelius posted 1 patch 2 days, 10 hours ago
fs/hpfs/ea.c | 57 ++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 49 insertions(+), 8 deletions(-)
[PATCH] hpfs: validate external EA record lengths
Posted by Samuel Moelius 2 days, 10 hours ago
HPFS stores the length of the external extended-attribute list in
fnode->ea_size_l, but the external EA walkers only check that four bytes
of header remain before trusting the record's namelen and valuelen
fields.

A crafted image can declare an external EA list of four bytes while the
first record header asks for a much longer name/value area. Reject
records whose computed size does not fit in the declared external EA
list before reading or advancing over the variable-length fields. Also
reject indirect EA headers whose value length is not the required 8-byte
sector/length payload before reading that payload.

Keep the external EA list length unsigned while walking it so the
on-disk u32 length is not narrowed into a signed int before these bounds
checks.

Assisted-by: Codex:gpt-5.5-cyber-preview
Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
---
 fs/hpfs/ea.c | 57 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/fs/hpfs/ea.c b/fs/hpfs/ea.c
index 4664f9ab06ee..ecd630604444 100644
--- a/fs/hpfs/ea.c
+++ b/fs/hpfs/ea.c
@@ -9,6 +9,29 @@
 
 #include "hpfs_fn.h"
 
+static bool hpfs_ea_ext_fits(struct super_block *s, secno a, int ano,
+			     unsigned int pos, unsigned int len,
+			     unsigned int size)
+{
+	if (pos > len || size > len - pos) {
+		hpfs_error(s, "EA record doesn't fit, %s %08x, pos %08x, len %08x",
+			   ano ? "anode" : "sectors", a, pos, len);
+		return false;
+	}
+	return true;
+}
+
+static bool hpfs_ea_indirect_ok(struct super_block *s, secno a, int ano,
+				unsigned int pos, struct extended_attribute *ea)
+{
+	if (ea_indirect(ea) && ea_valuelen(ea) != 8) {
+		hpfs_error(s, "ea_indirect(ea) set while ea->valuelen!=8, %s %08x, pos %08x",
+			   ano ? "anode" : "sectors", a, pos);
+		return false;
+	}
+	return true;
+}
+
 /* Remove external extended attributes. ano specifies whether a is a 
    direct sector where eas starts or an anode */
 
@@ -24,12 +47,12 @@ void hpfs_ea_ext_remove(struct super_block *s, secno a, int ano, unsigned len)
 			return;
 		}
 		if (hpfs_ea_read(s, a, ano, pos, 4, ex)) return;
+		if (!hpfs_ea_ext_fits(s, a, ano, pos, len,
+				      5 + ea->namelen + ea_valuelen(ea)))
+			return;
+		if (!hpfs_ea_indirect_ok(s, a, ano, pos, ea))
+			return;
 		if (ea_indirect(ea)) {
-			if (ea_valuelen(ea) != 8) {
-				hpfs_error(s, "ea_indirect(ea) set while ea->valuelen!=8, %s %08x, pos %08x",
-					ano ? "anode" : "sectors", a, pos);
-				return;
-			}
 			if (hpfs_ea_read(s, a, ano, pos + 4, ea->namelen + 9, ex+4))
 				return;
 			hpfs_ea_remove(s, ea_sec(ea), ea_in_anode(ea), ea_len(ea));
@@ -75,7 +98,8 @@ int hpfs_read_ea(struct super_block *s, struct fnode *fnode, char *key,
 		char *buf, int size)
 {
 	unsigned pos;
-	int ano, len;
+	unsigned int len;
+	int ano;
 	secno a;
 	char ex[4 + 255 + 1 + 8];
 	struct extended_attribute *ea;
@@ -102,6 +126,11 @@ int hpfs_read_ea(struct super_block *s, struct fnode *fnode, char *key,
 			return -EIO;
 		}
 		if (hpfs_ea_read(s, a, ano, pos, 4, ex)) return -EIO;
+		if (!hpfs_ea_ext_fits(s, a, ano, pos, len,
+				      5 + ea->namelen + ea_valuelen(ea)))
+			return -EIO;
+		if (!hpfs_ea_indirect_ok(s, a, ano, pos, ea))
+			return -EIO;
 		if (hpfs_ea_read(s, a, ano, pos + 4, ea->namelen + 1 + (ea_indirect(ea) ? 8 : 0), ex + 4))
 			return -EIO;
 		if (!strcmp(ea->name, key)) {
@@ -131,7 +160,8 @@ char *hpfs_get_ea(struct super_block *s, struct fnode *fnode, char *key, int *si
 {
 	char *ret;
 	unsigned pos;
-	int ano, len;
+	unsigned int len;
+	int ano;
 	secno a;
 	struct extended_attribute *ea;
 	struct extended_attribute *ea_end = fnode_end_ea(fnode);
@@ -160,6 +190,11 @@ char *hpfs_get_ea(struct super_block *s, struct fnode *fnode, char *key, int *si
 			return NULL;
 		}
 		if (hpfs_ea_read(s, a, ano, pos, 4, ex)) return NULL;
+		if (!hpfs_ea_ext_fits(s, a, ano, pos, len,
+				      5 + ea->namelen + ea_valuelen(ea)))
+			return NULL;
+		if (!hpfs_ea_indirect_ok(s, a, ano, pos, ea))
+			return NULL;
 		if (hpfs_ea_read(s, a, ano, pos + 4, ea->namelen + 1 + (ea_indirect(ea) ? 8 : 0), ex + 4))
 			return NULL;
 		if (!strcmp(ea->name, key)) {
@@ -193,7 +228,8 @@ void hpfs_set_ea(struct inode *inode, struct fnode *fnode, const char *key,
 	fnode_secno fno = inode->i_ino;
 	struct super_block *s = inode->i_sb;
 	unsigned pos;
-	int ano, len;
+	unsigned int len;
+	int ano;
 	secno a;
 	unsigned char h[4];
 	struct extended_attribute *ea;
@@ -221,6 +257,11 @@ void hpfs_set_ea(struct inode *inode, struct fnode *fnode, const char *key,
 			return;
 		}
 		if (hpfs_ea_read(s, a, ano, pos, 4, ex)) return;
+		if (!hpfs_ea_ext_fits(s, a, ano, pos, len,
+				      5 + ea->namelen + ea_valuelen(ea)))
+			return;
+		if (!hpfs_ea_indirect_ok(s, a, ano, pos, ea))
+			return;
 		if (hpfs_ea_read(s, a, ano, pos + 4, ea->namelen + 1 + (ea_indirect(ea) ? 8 : 0), ex + 4))
 			return;
 		if (!strcmp(ea->name, key)) {
-- 
2.43.0