linux-kernel - [PATCH] printk: Check real user/group id for %pK

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-next>] [day] [month] [year] [list]

Message-ID: <5248AB37.7000100@gmail.com>
Date:	Mon, 30 Sep 2013 08:35:35 +1000
From:	Ryan Mallon <rmallon@...il.com>
To:	LKML <linux-kernel@...r.kernel.org>, linux@...izon.com,
	Jiri Kosina <jkosina@...e.cz>, eldad@...refinery.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	Dan Rosenberg <dan.j.rosenberg@...il.com>
CC:	LKML <linux-kernel@...r.kernel.org>
Subject: [PATCH] printk: Check real user/group id for %pK

Some setuid binaries will allow reading of files which have read permission by the real user id. This is problematic with files which use %pK because the contents of the file is different when opened as root, and displaying the contents may leak kernel pointer values.
    
This happens for example with the setuid pppd application on Ubuntu 12.04:
    
  $ head -1 /proc/kallsyms
  00000000 T startup_32
    
  $ pppd file /proc/kallsyms
  pppd: In file /proc/kallsyms: unrecognized option 'c1000000'
    
This will only leak the pointer value from the first line, but other setuid binaries may leak more information.
    
Fix this by adding a check that in addition to the current process having CAP_SYSLOG, that effective user and group ids are equal to the real ids. If a setuid binary reads the contents of a file which uses %pK then the pointer values will be printed as NULL if the real user is unprivileged.

Signed-off-by: Ryan Mallon <rmallon@...il.com>
---

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 26559bd..b1cd14d 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1312,10 +1312,24 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 				spec.field_width = default_width;
 			return string(buf, end, "pK-error", spec);
 		}
-		if (!((kptr_restrict == 0) ||
-		      (kptr_restrict == 1 &&
-		       has_capability_noaudit(current, CAP_SYSLOG))))
-			ptr = NULL;
+
+		/*
+		 * If kptr_restrict is set to 2, then %pK always prints as
+		 * NULL. If it is set to 1, then only print the real pointer
+		 * value if the current proccess has CAP_SYSLOG and is running
+		 * with the same credentials it started with. We don't want
+		 * badly written setuid binaries being able to read the real
+		 * pointers on behalf of unprivileged users.
+		 */
+		{
+			const struct cred *cred = current_cred();
+
+			if (kptr_restrict == 2 || (kptr_restrict == 1 &&
+			     (!has_capability_noaudit(current, CAP_SYSLOG) ||
+			      !uid_eq(cred->euid, cred->uid) ||
+			      !gid_eq(cred->egid, cred->gid))))
+				ptr = NULL;
+		}
 		break;
 	case 'N':
 		switch (fmt[1]) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/