Attachment 599485 Details for Bug 840920 – Proposed patch to resolve issues in pcp namelist pdu handling

[patch] Proposed patch to resolve issues in pcp namelist pdu handling

pcp_namelist.patch (text/plain), 4.44 KB, created by Nathan Scott on 2012-07-21 04:06:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nathan Scott

Created: 2012-07-21 04:06:28 UTC

Size: 4.44 KB

patch

obsolete

>diff --git a/src/libpcp/src/p_pmns.c b/src/libpcp/src/p_pmns.c
>index a2908a0..f749fb6 100644
>--- a/src/libpcp/src/p_pmns.c
>+++ b/src/libpcp/src/p_pmns.c
>@@ -284,77 +284,100 @@ __pmSendNameList(int fd, int from, int numnames, char *namelist[],
>  * statuslist is optional ... if NULL, no status values will be returned
>  */
> int
>-__pmDecodeNameList(__pmPDU *pdubuf, int *numnames, 
>+__pmDecodeNameList(__pmPDU *pdubuf, int *numnamesp,
>                   char*** namelist, int** statuslist)
> {
>     namelist_t	*namelist_pdu;
>+    char 	*pdu_end;
>     char        **names;
>+    char	*dest, *dest_end;
>     int		*status = NULL;
>-    int 	need;
>-    int		numstatus;
>+    int 	namesize, numnames;
>+    int 	statussize, numstatus;
>+    int 	nstrbytes;
>+    int		namelen;
>+    int		i, j;
> 
>     namelist_pdu = (namelist_t *)pdubuf;
>+    pdu_end = (char *)pdubuf + namelist_pdu->hdr.len;
> 
>     *namelist = NULL;
>     if (statuslist != NULL)
> 	*statuslist = NULL;
> 
>-    *numnames = ntohl(namelist_pdu->numnames);
>+    numnames = ntohl(namelist_pdu->numnames);
>     numstatus = ntohl(namelist_pdu->numstatus);
>+    nstrbytes = ntohl(namelist_pdu->nstrbytes);
> 
>-    if (*numnames == 0)
>+    if (numnames == 0)
> 	return 0;
> 
>-    if (*numnames < 0 || numstatus < 0)
>-	/* should not happen */
>+    /* validity checks - none of these conditions should happen */
>+    if (numnames < 0 || numstatus < 0 || nstrbytes < 0)
> 	return PM_ERR_IPC;
> 
>     /* need space for name ptrs and the name characters */
>-    need = *numnames * ((int)sizeof(char*)) + ntohl(namelist_pdu->nstrbytes);
>-    if ((names = (char**)malloc(need)) == NULL)
>+    if (numnames >= (INT_MAX - nstrbytes) / (int)sizeof(char *))
>+	return PM_ERR_IPC;
>+    namesize = numnames * ((int)sizeof(char*)) + nstrbytes;
>+    if ((names = (char**)malloc(namesize)) == NULL)
> 	return -oserror();
> 
>     /* need space for status values */
>     if (statuslist != NULL && numstatus > 0) {
>-	need = numstatus * (int)sizeof(int);
>-	if ((status = (int*)malloc(need)) == NULL) {
>+	if (numstatus >= INT_MAX / (int)sizeof(int))
>+	    goto corrupt;
>+	statussize = numstatus * (int)sizeof(int);
>+	if ((status = (int*)malloc(statussize)) == NULL) {
> 	    free(names);
> 	    return -oserror();
> 	}
>     }
> 
>+    dest = (char*)&names[numnames];
>+    dest_end = (char*)names + namesize;
>+
>     /* copy over ptrs and characters */
>     if (numstatus == 0) {
>-	int	i, j = 0;
>-	char	*dest = (char*)&names[*numnames];
> 	name_t	*np;
>-	int	namelen;
> 
>-        for (i = 0; i < *numnames; i++) {
>+	for (i = j = 0; i < numnames; i++) {
> 	    np = (name_t*)&namelist_pdu->names[j/sizeof(__pmPDU)];
> 	    names[i] = dest;
>+
>+	    if ((char *)np + sizeof(name_t) > pdu_end)
>+		goto corrupt;
> 	    namelen = ntohl(np->namelen);
>+	    if ((char *)np + sizeof(np->namelen) + namelen > pdu_end)
>+		goto corrupt;
>+	    if ((namelen + 1) > (dest_end - dest))
>+		goto corrupt;
> 
> 	    memcpy(dest, np->name, namelen);
>-	    *(dest + namelen) = '\0';
>-	    dest += namelen + 1; 
>+	    dest += namelen; 
>+	    *dest = '\0';
>+	    dest++;
> 
> 	    j += sizeof(namelen) + PM_PDU_SIZE_BYTES(namelen);
> 	}
>     }
>     else { /* status fields included in the PDU */
>-	int		i, j = 0;
>-        char		*dest = (char*)&names[*numnames];
> 	name_status_t	*np;
>-	int		namelen;
> 
>-	for (i = 0; i < *numnames; i++) {
>+	for (i = j = 0; i < numnames; i++) {
> 	    np = (name_status_t*)&namelist_pdu->names[j/sizeof(__pmPDU)];
> 	    names[i] = dest;
>+
>+	    if ((char *)np + sizeof(name_status_t) > pdu_end)
>+		goto corrupt;
> 	    namelen = ntohl(np->namelen);
>+	    if ((char *)np + sizeof(np->namelen) + sizeof(np->status) + namelen > pdu_end)
>+		goto corrupt;
>+	    if ((namelen + 1) > (dest_end - dest))
>+		goto corrupt;
>+
> 	    if (status != NULL)
> 		status[i] = ntohl(np->status);
>-
> 	    memcpy(dest, np->name, namelen);
> 	    *(dest + namelen) = '\0';
> 	    dest += namelen + 1; 
>@@ -366,7 +389,7 @@ __pmDecodeNameList(__pmPDU *pdubuf, int *numnames,
> #ifdef PCP_DEBUG
>     if (pmDebug & DBG_TRACE_PMNS) {
> 	fprintf(stderr, "__pmDecodeNameList\n");
>-	__pmDumpNameList(stderr, *numnames, names);
>+	__pmDumpNameList(stderr, numnames, names);
> 	if (status != NULL)
> 	    __pmDumpStatusList(stderr, numstatus, status);
>     }
>@@ -375,7 +398,14 @@ __pmDecodeNameList(__pmPDU *pdubuf, int *numnames,
>     *namelist = names;
>     if (statuslist != NULL)
> 	*statuslist = status;
>-    return *numnames;
>+    *numnamesp = numnames;
>+    return numnames;
>+
>+corrupt:
>+    if (status != NULL)
>+	free(status);
>+    free(names);
>+    return PM_ERR_IPC;
> }
> 
>

Actions: View | Diff

Attachments on bug 840920: 599485 | 599886 | 600251 | 601724