linux-ext4 - Re: [PATCH] ext4: limit external inode xattrs to XATTR_SIZE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180331002209.GG9300@thunk.org>
Date:   Fri, 30 Mar 2018 20:22:10 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Eric Biggers <ebiggers3@...il.com>
Cc:     Andreas Dilger <adilger@...ger.ca>,
        linux-ext4 <linux-ext4@...r.kernel.org>,
        Wen Xu <wen.xu@...ech.edu>, Eric Biggers <ebiggers@...gle.com>
Subject: Re: [PATCH] ext4: limit external inode xattrs to XATTR_SIZE_MAX

I've done the following which should hopefully make you happier and
things clearer.  First of all, I've taken the extra checks and moved
out of this commit.  So it now looks pretty much like your original
proposed patch.

Then I've added two separate patches to add better bounds checking to
the xattr read and find path.  There is almost certainly more paranoia
checks that could be added later --- in particular in the xattr set
codepaths --- but this is the low-hanging fruit to make life more
interesting for people doing research in file system fuzzing tools.  :-)

					- Ted

>From ce3fd194fcc6fbdc00ce095a852f22df97baa401 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@...gle.com>
Date: Thu, 29 Mar 2018 14:31:42 -0400
Subject: [PATCH] ext4: limit xattr size to INT_MAX

ext4 isn't validating the sizes of xattrs where the value of the xattr
is stored in an external inode.  This is problematic because
->e_value_size is a u32, but ext4_xattr_get() returns an int.  A very
large size is misinterpreted as an error code, which ext4_get_acl()
translates into a bogus ERR_PTR() for which IS_ERR() returns false,
causing a crash.

Fix this by validating that all xattrs are <= INT_MAX bytes.

This issue has been assigned CVE-2018-1095.

https://bugzilla.kernel.org/show_bug.cgi?id=199185
https://bugzilla.redhat.com/show_bug.cgi?id=1560793

Reported-by: Wen Xu <wen.xu@...ech.edu>
Signed-off-by: Eric Biggers <ebiggers@...gle.com>
Signed-off-by: Theodore Ts'o <tytso@....edu>
Cc: stable@...r.kernel.org
Fixes: e50e5129f384 ("ext4: xattr-in-inode support")
---
 fs/ext4/xattr.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 63656dbafdc4..2077d87b09f2 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -195,10 +195,13 @@ ext4_xattr_check_entries(struct ext4_xattr_entry *entry, void *end,
 
 	/* Check the values */
 	while (!IS_LAST_ENTRY(entry)) {
-		if (entry->e_value_size != 0 &&
-		    entry->e_value_inum == 0) {
+		u32 size = le32_to_cpu(entry->e_value_size);
+
+		if (size > INT_MAX)
+			return -EFSCORRUPTED;
+
+		if (size != 0 && entry->e_value_inum == 0) {
 			u16 offs = le16_to_cpu(entry->e_value_offs);
-			u32 size = le32_to_cpu(entry->e_value_size);
 			void *value;
 
 			/*
-- 
2.16.1.72.g5be1f00a9a