netdev - Re: [Security] [SECURITY] DECnet: need to validate user data and access data?

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]

Date:	Tue, 22 Mar 2011 15:42:22 +0800
From:	Eugene Teo <eugeneteo@...nel.org>
To:	Dan Rosenberg <drosenberg@...curity.com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org, security@...nel.org,
	linux-decnet-user@...ts.sourceforge.net
Subject: Re: [Security] [SECURITY] DECnet: need to validate user data and
 access data?

Cc'ed the decnet list. Looks like it's still active even though the
status is orphan.

On Tue, Mar 22, 2011 at 7:41 AM, Dan Rosenberg <drosenberg@...curity.com> wrote:
> In net/decnet/af_decnet.c, in the dn_access_copy() and dn_user_copy()
> functions, which are called from dn_connect(), length values are
> retrieved from incoming skb data and used as size values to copy
> functions:
>
> static void dn_access_copy(struct sk_buff *skb, struct accessdata_dn *acc)
> {
>        unsigned char *ptr = skb->data;
>
>        acc->acc_userl = *ptr++;
>        memcpy(&acc->acc_user, ptr, acc->acc_userl);
>        ptr += acc->acc_userl;
>
>        acc->acc_passl = *ptr++;
>        memcpy(&acc->acc_pass, ptr, acc->acc_passl);
>        ptr += acc->acc_passl;
>
>        acc->acc_accl = *ptr++;
>        memcpy(&acc->acc_acc, ptr, acc->acc_accl);
>
>        skb_pull(skb, acc->acc_accl + acc->acc_passl + acc->acc_userl + 3);
>
> }
>
> static void dn_user_copy(struct sk_buff *skb, struct optdata_dn *opt)
> {
>        unsigned char *ptr = skb->data;
>        u16 len = *ptr++; /* yes, it's 8bit on the wire */
>
>        BUG_ON(len > 16); /* we've checked the contents earlier */
>        opt->opt_optl   = cpu_to_le16(len);
>        opt->opt_status = 0;
>        memcpy(opt->opt_data, ptr, len);
>        skb_pull(skb, len + 1);
> }
>
>
> Despite the BUG_ON and comment suggesting these lengths have been
> validated, I don't think this is actually the case - it looks like these
> fields are validated for outbound data, but I see no validation for
> inbound data (unless I'm mistaken, which is entirely possible).  If this
> is the case, this can allow remote attackers to cause controllable heap
> corruption.  I'd appreciate it if someone who knows this protocol better
> than I do took a look at this and implemented appropriate error handling
> if it needs it.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html