Attachment 603365 Details for Bug 841240 – Revised PCP instance request PDU decoding patch

[patch] Revised PCP instance request PDU decoding patch

pcp_instreq (text/plain), 2.64 KB, created by Nathan Scott on 2012-08-09 23:00:02 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nathan Scott

Created: 2012-08-09 23:00:02 UTC

Size: 2.64 KB

patch

obsolete

>Fix heap buffer overflow in __pmDecodeInstanceReq routine
>
>__pmDecodeInstanceReq does not check the namelen field against the
>PDU length, leading to a read overflow.  Furthermore, namelen is not
>validated prior to a malloc call.
>
>Original report and fixes reviewed by Florian Weimer of the Red Hat
>Security Team.  Red Hat bugzilla bug #841240.
>
>Security advisory CVE-2012-3418.
>
>Index: pcp/src/libpcp/src/p_instance.c
>===================================================================
>--- pcp.orig/src/libpcp/src/p_instance.c	2012-08-10 07:54:14.617709290 +1000
>+++ pcp/src/libpcp/src/p_instance.c	2012-08-10 07:54:30.922718944 +1000
>@@ -75,22 +75,35 @@ int
> __pmDecodeInstanceReq(__pmPDU *pdubuf, __pmTimeval *when, pmInDom *indom, int *inst, char **name)
> {
>     instance_req_t	*pp;
>+    char		*pdu_end;
>+    int			namelen;
> 
>     pp = (instance_req_t *)pdubuf;
>+    pdu_end = (char *)pdubuf + pp->hdr.len;
>+
>+    if (pdu_end - (char *)pp < sizeof(instance_req_t) - sizeof(pp->name))
>+	return PM_ERR_IPC;
>+
>     when->tv_sec = ntohl(pp->when.tv_sec);
>     when->tv_usec = ntohl(pp->when.tv_usec);
>     *indom = __ntohpmInDom(pp->indom);
>     *inst = ntohl(pp->inst);
>-    pp->namelen = ntohl(pp->namelen);
>-    if (pp->namelen) {
>-	if ((*name = (char *)malloc(pp->namelen+1)) == NULL)
>+    namelen = ntohl(pp->namelen);
>+    if (namelen > 0) {
>+	if (namelen >= INT_MAX - 1 || namelen > pp->hdr.len)
>+	    return PM_ERR_IPC;
>+	if (pdu_end - (char *)pp < sizeof(instance_req_t) - sizeof(pp->name) + namelen)
>+	    return PM_ERR_IPC;
>+	if ((*name = (char *)malloc(namelen+1)) == NULL)
> 	    return -oserror();
>-	strncpy(*name, pp->name, pp->namelen);
>-	(*name)[pp->namelen] = '\0';
>+	strncpy(*name, pp->name, namelen);
>+	(*name)[namelen] = '\0';
>     }
>-    else
>+    else if (namelen < 0) {
>+	return PM_ERR_IPC;
>+    } else {
> 	*name = NULL;
>-
>+    }
>     return 0;
> }
> 
>Index: pcp/src/pmcd/src/dopdus.c
>===================================================================
>--- pcp.orig/src/pmcd/src/dopdus.c	2012-08-10 07:53:52.093718011 +1000
>+++ pcp/src/pmcd/src/dopdus.c	2012-08-10 07:53:40.977708902 +1000
>@@ -282,7 +282,7 @@ DoDesc(ClientInfo *cp, __pmPDU *pb)
> int
> DoInstance(ClientInfo *cp, __pmPDU* pb)
> {
>-    int			sts = 0, s;
>+    int			sts, s;
>     __pmTimeval		when;
>     pmInDom		indom;
>     int			inst;
>@@ -291,7 +291,9 @@ DoInstance(ClientInfo *cp, __pmPDU* pb)
>     AgentInfo		*ap;
>     int			fdfail = -1;
> 
>-    __pmDecodeInstanceReq(pb, &when, &indom, &inst, &name);
>+    sts = __pmDecodeInstanceReq(pb, &when, &indom, &inst, &name);
>+    if (sts < 0)
>+	return sts;
>     if (when.tv_sec != 0 || when.tv_usec != 0) {
> 	/*
> 	 * we have no idea how to do anything but current, yet!

Actions: View | Diff

Attachments on bug 841240: 600703 | 603365