xorl %eax, %eax

Archive for the ‘vulnerabilities’ Category

CVE-2018-0739: OpenSSL ASN.1 stack overflow

leave a comment »

This was a vulnerability discovered by Google’s OSS-Fuzz project and it was fixed by Matt Caswell of the OpenSSL development team. The vulnerability affects OpenSSL releases prior to 1.0.2o and 1.1.0h and based on OpenSSL team’s assessment, this cannot be triggered via SSL/TLS but constructed ASN.1 types with support for recursive definitions, such as PKCS7 can be used to trigger it.

/*
 * Decode an item, taking care of IMPLICIT tagging, if any. If 'opt' set and
 * tag mismatch return -1 to handle OPTIONAL
 */

static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in,
                               long len, const ASN1_ITEM *it,
                               int tag, int aclass, char opt, ASN1_TLC *ctx)
{
    const ASN1_TEMPLATE *tt, *errtt = NULL;
    const ASN1_EXTERN_FUNCS *ef;
    const ASN1_AUX *aux = it->funcs;
    ASN1_aux_cb *asn1_cb;
    const unsigned char *p = NULL, *q;
    unsigned char oclass;
    char seq_eoc, seq_nolen, cst, isopt;
    long tmplen;
    int i;
    int otag;
    int ret = 0;
    ASN1_VALUE **pchptr;
    if (!pval)
        return 0;
    if (aux && aux->asn1_cb)
        asn1_cb = aux->asn1_cb;
    else
        asn1_cb = 0;

    switch (it->itype) {
   ...
    return 0;
}

What you see above is a snippet of crypto/asn1/tasn_dec.c where the decoding ASN.1 function asn1_item_embed_d2i() is located. Neither this function nor any of its callers had any check for recursive definitions. This means that given a malicious PKCS7 file this decoding routine will keep on trying to decode them until the process will run out of stack space.

To fix this, a new error case was added in crypto/asn1/asn1.h header file named ASN1_R_NESTED_TOO_DEEP. If we have a look at crypto/asn1/asn1_err.c we can see that the new error code is equivalent to the “nested too deep” error message.

     {ERR_REASON(ASN1_R_NESTED_ASN1_STRING), "nested asn1 string"},
+    {ERR_REASON(ASN1_R_NESTED_TOO_DEEP), "nested too deep"},
     {ERR_REASON(ASN1_R_NON_HEX_CHARACTERS), "non hex characters"},

Similarly, a new constant (ASN1_MAX_CONSTRUCTED_NEST) definition was added which is used to define the maximum amount of recursive invocations of asn1_item_embed_d2i() function. You can see the new definition in crypto/asn1/tasn_dec.c.

 #include <openssl/err.h>

/*
 * Constructed types with a recursive definition (such as can be found in PKCS7)
 * could eventually exceed the stack given malicious input with excessive
 * recursion. Therefore we limit the stack depth. This is the maximum number of
 * recursive invocations of asn1_item_embed_d2i().
 */
#define ASN1_MAX_CONSTRUCTED_NEST 30

 static int asn1_check_eoc(const unsigned char **in, long len);

Lastly, the asn1_item_embed_d2i() function itself was modified to have a new integer argument “depth” which is used as a counter for each iteration. You can see how check is performed before entering the switch clause here.

        asn1_cb = 0; 

    if (++depth > ASN1_MAX_CONSTRUCTED_NEST) { 
        ASN1err(ASN1_F_ASN1_ITEM_EMBED_D2I, ASN1_R_NESTED_TOO_DEEP);
        goto err;
    }

    switch (it->itype) {
    case ASN1_ITYPE_PRIMITIVE:

Similarly, all calling functions on OpenSSL have been updated to ensure that the new argument is used as intended. The official security advisory describes the above vulnerability like this.

Constructed ASN.1 types with a recursive definition could exceed the stack (CVE-2018-0739)
==========================================================================================

Severity: Moderate

Constructed ASN.1 types with a recursive definition (such as can be found in
PKCS7) could eventually exceed the stack given malicious input with
excessive recursion. This could result in a Denial Of Service attack. There are
no such structures used within SSL/TLS that come from untrusted sources so this
is considered safe.

OpenSSL 1.1.0 users should upgrade to 1.1.0h
OpenSSL 1.0.2 users should upgrade to 1.0.2o
 
This issue was reported to OpenSSL on 4th January 2018 by the OSS-fuzz project.
The fix was developed by Matt Caswell of the OpenSSL development team.

Written by xorl

March 30, 2018 at 19:41

Posted in vulnerabilities

CVE-2017-12762: Linux kernel isdn_net IOCTL memory corruption

leave a comment »

Recently I came across this report which is kind of sad since this was one nice and funny 0day that had been around for very long time. However, in this post I will only talk about the vulnerability since no exploit has been publicly disclosed yet. The vulnerability is in I4L (ISDN 4 Linux) and starts with the IIOCNETASL (Create slave interface) IOCTL command which is in drivers/isdn/i4l/isdn_common.c as shown below.

static int
isdn_ioctl(struct file *file, uint cmd, ulong arg)
{
   ...
		switch (cmd) {
   ...

		case IIOCNETASL:
			/* Add a slave to a network-interface */
			if (arg) {
				if (copy_from_user(bname, argp, sizeof(bname) - 1))
					return -EFAULT;
			} else
				return -EINVAL;
			ret = mutex_lock_interruptible(&dev->mtx);
			if (ret) return ret;
			if ((s = isdn_net_newslave(bname))) {
				if (copy_to_user(argp, s, strlen(s) + 1)) {
   ...
}

Basically this is accessible via /dev/isdnctrl ISDN control device and ioctl(2) system call using IIOCNETASL command. As you can see in the above snippet, it uses copy_from_user() to get the user controlled buffer and store it in “bname” which is a stack allocated buffer that you can see how it was defined below.

	union iocpar {
		char name[10];
		char bname[22];
		isdn_ioctl_struct iocts;
		isdn_net_ioctl_phone phone;
		isdn_net_ioctl_cfg cfg;
	} iocpar;
	void __user *argp = (void __user *)arg;

#define name  iocpar.name
#define bname iocpar.bname

Later on we see that the user derived “bname” buffer is passed to isdn_net_newslave() which is a function defined in drivers/isdn/i4l/isdn_net.c. Here is this function.

char *
isdn_net_newslave(char *parm)
{
	char *p = strchr(parm, ',');
	isdn_net_dev *n;
	char newname[10];

	if (p) {
		/* Slave-Name MUST not be empty */
		if (!strlen(p + 1))
			return NULL;
		strcpy(newname, p + 1);
		*p = 0;
   ...
}

Here we can see that the only check on the user derived “p” pointer is that it is not empty. Then it uses strcpy() to copy the contents of it to “newname” which is a stack buffer with size of 10 Bytes. This is like a 90s textbook stack buffer overflow. In August 2017 it was reported and patched by Annie Cherkaev by replacing strcpy() with strscpy() which ensures that the copy will not exceed “newname” buffer’s limits. The patch is the following.

--- a/drivers/isdn/i4l/isdn_common.c
+++ b/drivers/isdn/i4l/isdn_common.c
@@ -1379,6 +1379,7 @@  isdn_ioctl(struct file *file, uint cmd,
 			if (arg) {
 				if (copy_from_user(bname, argp, sizeof(bname) - 1))
 					return -EFAULT;
+				bname[sizeof(bname)-1] = 0;
 			} else
 				return -EINVAL;
 			ret = mutex_lock_interruptible(&dev->mtx);
--- a/drivers/isdn/i4l/isdn_net.c
+++ b/drivers/isdn/i4l/isdn_net.c
@@ -2611,10 +2611,9 @@  isdn_net_newslave(char *parm)
 	char newname[10];
 
 	if (p) {
-		/* Slave-Name MUST not be empty */
-		if (!strlen(p + 1))
+		/* Slave-Name MUST not be empty or overflow 'newname' */
+		if (strscpy(newname, p + 1, sizeof(newname)) <= 0)
 			return NULL;
-		strcpy(newname, p + 1);
 		*p = 0;
 		/* Master must already exist */
 		if (!(n = isdn_net_findif(parm)))

This is kind of sad, not because this is a useful 0day but because it had been around for years. Me and some friends had this 0day literally from 2007 so it is kind of sad seeing it dying quietly like this. In any case, I will not go into how to exploit it but it is a nice trivial vulnerability if you want to play around and practice your Linux kernel stack memory corruption exploitation techniques.

Written by xorl

February 24, 2018 at 02:37

Posted in vulnerabilities

CVE-2017-17046: Xen ARM Information Leak

leave a comment »

This vulnerability was released with XSA-245 security advisory by Julien Grall of ARM. The issue is simply the lack of cleaning up memory which means that one domain might end up having access to uncleaned memory of another domain after reboots (soft or hard). This vulnerability affects only ARM because they do not support NUMA and, as J. Grall explained, specific regions (like the Dom0 kernel) are marked as reserved until the hardware domain is built and they are subsequently copied to its memory. To fix this, xen/common/page_alloc.c was modified as you see below.

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 0b9f6cc6df..fbe5a8af39 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1700,6 +1700,16 @@ static void init_heap_pages(
 {
     unsigned long i;
 
+    /*
+     * Some pages may not go through the boot allocator (e.g reserved
+     * memory at boot but released just after --- kernel, initramfs,
+     * etc.).
+     * Update first_valid_mfn to ensure those regions are covered.
+     */
+    spin_lock(&heap_lock);
+    first_valid_mfn = min_t(unsigned long, page_to_mfn(pg), first_valid_mfn);
+    spin_unlock(&heap_lock);
+
     for ( i = 0; i < nr_pages; i++ )
     {
         unsigned int nid = phys_to_nid(page_to_maddr(pg+i));

And in the same source code file, the equivalent helpers were added to make the above change feasible.

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index fbe5a8af39..472c6fe329 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -192,7 +192,11 @@ PAGE_LIST_HEAD(page_broken_list);
  * BOOT-TIME ALLOCATOR
  */
 
-static unsigned long __initdata first_valid_mfn = ~0UL;
+/*
+ * first_valid_mfn is exported because it is use in ARM specific NUMA
+ * helpers. See comment in asm-arm/numa.h.
+ */
+unsigned long first_valid_mfn = ~0UL;
 
 static struct bootmem_region {
     unsigned long s, e; /* MFNs @s through @e-1 inclusive are free */
diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
index a2c1a3476d..3e7384da9e 100644
--- a/xen/include/asm-arm/numa.h
+++ b/xen/include/asm-arm/numa.h
@@ -12,9 +12,15 @@ static inline __attribute__((pure)) nodeid_t phys_to_nid(paddr_t addr)
     return 0;
 }
 
+/*
+ * TODO: make first_valid_mfn static when NUMA is supported on Arm, this
+ * is required because the dummy helpers is using it.
+ */
+extern unsigned long first_valid_mfn;
+
 /* XXX: implement NUMA support */
-#define node_spanned_pages(nid) (total_pages)
-#define node_start_pfn(nid) (pdx_to_pfn(frametable_base_pdx))
+#define node_spanned_pages(nid) (max_page - first_valid_mfn)
+#define node_start_pfn(nid) (first_valid_mfn)
 #define __node_distance(a, b) (20)

Written by xorl

January 22, 2018 at 10:15

Posted in vulnerabilities

Linux kernel RDMA NULL pointer dereference

leave a comment »

While reading through Linux kernel’s ChangeLog I came across this one. I was unable to find any CVE ID for this vulnerability reported by Håkon Bugge on 6 December 2017. The vulnerability was discovered by Google’s syzkaller kernel fuzzer and reported in syzkaller719569. The vulnerable code starts in the setsockopt() system call implementation for RDS (Reliable Datagram Sockets). This code is located at net/rds/af_rds.c and here is the code path we are interested in.

static int rds_setsockopt(struct socket *sock, int level, int optname,
			  char __user *optval, unsigned int optlen)
{
	struct rds_sock *rs = rds_sk_to_rs(sock->sk);
	int ret;
   ...
	case RDS_GET_MR:
		ret = rds_get_mr(rs, optval, optlen);
		break;
	case RDS_GET_MR_FOR_DEST:
		ret = rds_get_mr_for_dest(rs, optval, optlen);
		break;
   ...
}

Before we check those two options, it is important to see some specific value in “rds_sock” structure. This structure is defined in net/rds/rds.h header file. The key here is that “rs_transport” pointer in “rds_sock” structure can be NULL.

/**
 * struct rds_transport -  transport specific behavioural hooks
   ...
 */
   ...
struct rds_transport {
   ...
};

struct rds_sock {
   ...
	/*
	 * bound_addr used for both incoming and outgoing, no INADDR_ANY
	 * support.
	 */
   ...
	struct rds_transport    *rs_transport;
   ...
};

Back in rds_setsockopt(), there are two options with very similar code. The RDS_GET_MR and RDS_GET_MR_FOR_DES which will both result in a code that uses copy_from_user() to copy the “rds_get_mr_args” or “rds_get_mr_for_dest_args” structure to the kernel space, and then invoke __rds_rdma_map() routine to do the mapping.

int rds_get_mr(struct rds_sock *rs, char __user *optval, int optlen)
{
	struct rds_get_mr_args args;
   ...
	if (copy_from_user(&args, (struct rds_get_mr_args __user *)optval,
			   sizeof(struct rds_get_mr_args)))
		return -EFAULT;

	return __rds_rdma_map(rs, &args, NULL, NULL);
}

int rds_get_mr_for_dest(struct rds_sock *rs, char __user *optval, int optlen)
{
   ...
	if (copy_from_user(&args, (struct rds_get_mr_for_dest_args __user *)optval,
			   sizeof(struct rds_get_mr_for_dest_args)))
		return -EFAULT;
   ...
	return __rds_rdma_map(rs, &new_args, NULL, NULL);
}

If we now move to __rds_rdma_map() in net/rds/rdma.c we will quickly notice what’s the issue that triggers the vulnerability. The code will try to access the get_mr() callback function from the “rs_transport” pointer which as mentioned earlier can be unset (NULL), resulting in a NULL pointer dereference. This code path can be reached from the previously described setsockopt() system call options.

static int __rds_rdma_map(struct rds_sock *rs, struct rds_get_mr_args *args,
				u64 *cookie_ret, struct rds_mr **mr_ret)
{
   ...
	if (!rs->rs_transport->get_mr) {
		ret = -EOPNOTSUPP;
		goto out;
	}
   ...
	return ret;
}

The patch was to add an additional check that ensures that “rs_transport” is not NULL. If it’s NULL the __rds_rdma_map() will exit with “ENOTCONN” (Transport endpoint is not connected) error code.

diff --git a/net/rds/rdma.c b/net/rds/rdma.c
index 8886f15abe90..bc2f1e0977d6 100644
--- a/net/rds/rdma.c
+++ b/net/rds/rdma.c
@@ -183,7 +183,7 @@  static int __rds_rdma_map(struct rds_sock *rs, struct rds_get_mr_args *args,
 	long i;
 	int ret;
 
-	if (rs->rs_bound_addr == 0) {
+	if (rs->rs_bound_addr == 0 || !rs->rs_transport) {
 		ret = -ENOTCONN; /* XXX not a great errno */
 		goto out;
 	}

Written by xorl

December 18, 2017 at 23:19

Posted in vulnerabilities

CVE-2017-17450: Linux kernel nfnl_osf security bypass

leave a comment »

This vulnerability is identical to CVE-2017-17448. It was reported by Kevin Cernekee and it affects the OSF nsfnetlink helper in the same way as CVE-2017-17448 affected the connection tracking helper of NetFilter. Basically, the lack of “CAP_NET_ADMIN” capability checks means that any unprivileged user can execute commands like the following to create or delete namespaces and also obtaining “CAP_NET_ADMIN” capability in the created ones.

    vpnns -- nfnl_osf -f /tmp/pf.os
    vpnns -- nfnl_osf -f /tmp/pf.os -d

Similarly to CVE-2017-17448, the fix was to add the missing checks in the add and remove functions as you can see below.

diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c
index 36e14b1..a34f314 100644
--- a/net/netfilter/xt_osf.c
+++ b/net/netfilter/xt_osf.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 
+#include <linux/capability.h>
 #include <linux/if.h>
 #include <linux/inetdevice.h>
 #include <linux/ip.h>
@@ -70,6 +71,9 @@ static int xt_osf_add_callback(struct net *net, struct sock *ctnl,
 	struct xt_osf_finger *kf = NULL, *sf;
 	int err = 0;
 
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+
 	if (!osf_attrs[OSF_ATTR_FINGER])
 		return -EINVAL;
 
@@ -115,6 +119,9 @@ static int xt_osf_remove_callback(struct net *net, struct sock *ctnl,
 	struct xt_osf_finger *sf;
 	int err = -ENOENT;
 
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+
 	if (!osf_attrs[OSF_ATTR_FINGER])
 		return -EINVAL;

Written by xorl

December 10, 2017 at 13:53

Posted in vulnerabilities

CVE-2017-17448: Linux kernel cthelper security bypass

leave a comment »

This is a vulnerability reported by Kevin Cernekee on 3 December 2017 and it affects nfnl_cthelper of NeFilter. The nfnl_cthelper is a connection tracking helper for the user-space and its code resides in net/netfilter/nfnetlink_cthelper.c. What Kevin Cernekee noticed is that nfnetlink_rcv() (located at net/netfilter/nfnetlink.c) checks if the caller has the “CAP_NET_ADMIN” permission capability but there is a design flaw.

static void nfnetlink_rcv(struct sk_buff *skb)
{
   ...
	if (!netlink_net_capable(skb, CAP_NET_ADMIN)) {
		netlink_ack(skb, nlh, -EPERM, NULL);
		return;
	}
   ...
}

This code checks for the “CAP_NET_ADMIN” capability only in the namespace that owns the “skb” socket. On the other hand, the nfnl_cthelper is a user-space helper that is shared by all network namespaces. Since there are no “CAP_NET_ADMIN” capability checks in nfnl_cthelper it means that an unprivileged user can create namespaces, bypassing the “CAP_NET_ADMIN” capability check. Kevin Cernekee’s patch for this vulnerability was to add the missing capability checks on the “new”, “get”, and “del” functions of this helper. You can see the patch here.

diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
index 41628b393673..d33ce6d5ebce 100644
--- a/net/netfilter/nfnetlink_cthelper.c
+++ b/net/netfilter/nfnetlink_cthelper.c
@@ -17,6 +17,7 @@ 
 #include <linux/types.h>
 #include <linux/list.h>
 #include <linux/errno.h>
+#include <linux/capability.h>
 #include <net/netlink.h>
 #include <net/sock.h>
 
@@ -407,6 +408,9 @@  static int nfnl_cthelper_new(struct net *net, struct sock *nfnl,
 	struct nfnl_cthelper *nlcth;
 	int ret = 0;
 
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+
 	if (!tb[NFCTH_NAME] || !tb[NFCTH_TUPLE])
 		return -EINVAL;
 
@@ -611,6 +615,9 @@  static int nfnl_cthelper_get(struct net *net, struct sock *nfnl,
 	struct nfnl_cthelper *nlcth;
 	bool tuple_set = false;
 
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+
 	if (nlh->nlmsg_flags & NLM_F_DUMP) {
 		struct netlink_dump_control c = {
 			.dump = nfnl_cthelper_dump_table,
@@ -678,6 +685,9 @@  static int nfnl_cthelper_del(struct net *net, struct sock *nfnl,
 	struct nfnl_cthelper *nlcth, *n;
 	int j = 0, ret;
 
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+
 	if (tb[NFCTH_NAME])
 		helper_name = nla_data(tb[NFCTH_NAME]);

Written by xorl

December 10, 2017 at 13:46

Posted in vulnerabilities

CVE-2017-17085: Wireshark CIP Safety dissector integer overflow

leave a comment »

On 30 November 2017 Wireshark released their WNPA-SEC-2017-49 security advisory. The security advisory describes a vulnerability in the CIP Safety protocol dissector which is identified as DoS. The vulnerability was discovered by “Buildbot Builder” via fuzzing and the trigger PoC PCAP is available online (fuzz-2017-11-28-28119.pcap). The vulnerable code is in epan/dissectors/packet-cipsafety.c and you can see the exact code path below.

/* packet-cipsafety.c
 * Routines for CIP (Common Industrial Protocol) Safety dissection
 * CIP Safety Home: www.odva.org
   ...
 */
   ...
static void
dissect_cip_safety_data( proto_tree *tree, proto_item *item, tvbuff_t *tvb, int item_length, packet_info *pinfo)
{
   int base_length, io_data_size;
   gboolean multicast = (((pntoh32(pinfo->dst.data)) & 0xf0000000) == 0xe0000000);
   ...
   /* compute the base packet length to determine what is actual I/O data */
   base_length = multicast ? 12 : 6;
   ...
      case CIP_SAFETY_EXTENDED_FORMAT:
         if (item_length-base_length <= 2)
         {
   ...
            proto_tree_add_item(tree, hf_cipsafety_crc_s5_0, tvb, item_length-base_length+1, 1, ENC_LITTLE_ENDIAN);
            proto_tree_add_item(tree, hf_cipsafety_crc_s5_1, tvb, item_length-base_length+2, 1, ENC_LITTLE_ENDIAN);
            proto_tree_add_item(tree, hf_cipsafety_timestamp, tvb, item_length-base_length+3, 2, ENC_LITTLE_ENDIAN);
            proto_tree_add_item(tree, hf_cipsafety_crc_s5_2, tvb, item_length-base_length+5, 1, ENC_LITTLE_ENDIAN);
   ...
}

As you can see from the above code snippet, dissect_cip_safety_data() retrieves a signed integer (item_length) as an argument. Later on, it calculates “base_length”. Then, if the packet is CIP Safety in extended format it will do the comparison that you see above. If “item_length” results in a negative value it will enter the ‘if’ clause and try to invoke proto_tree_add_item() with a negative length variable. The latter will try to copy a large amounts (specifically equal to “item_length”) of data leading to a segmentation fault due to memory corruption. Below you can see the patch that Pascal Quantin developed to fix this vulnerability.

     /* compute the base packet length to determine what is actual I/O data */
     base_length = multicast ? 12 : 6;
	 
+    if (item_length <= base_length) {
+       expert_add_info(pinfo, item, &ei_mal_io);
+       return;
+    }
+

The above ensures that “item_length” will never be smaller than “base_length” to enter the dissection code. This should catch cases of “item_length” being negative.

Written by xorl

December 10, 2017 at 13:28

Posted in vulnerabilities