Bug 841240

Summary: __pmDecodeInstanceReq heap buffer overflow
Product: [Fedora] Fedora Reporter: Florian Weimer <fweimer>
Component: pcpAssignee: Ken McDonell <kenj>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 16CC: kenj, mgoodwin, nathans, security-response-team
Target Milestone: ---Keywords: Security
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: pcp-3.6.5 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-08-20 03:54:18 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 840765, 841698    
Attachments:
Description Flags
Resolve issues in decoding PCP instreq PDUs
none
Revised PCP instance request PDU decoding patch none

Description Florian Weimer 2012-07-18 13:44:32 UTC 
__pmDecodeInstanceReq does not check the namelen field against the PDU length, leading to a read overflow.  Furthermore, if namelen is 0xFFFFFFFF, malloc(0) is called, followed by a strncpy() call to that pointer.  The copied string is controlled by the attacker.

This is exposed through pmcd without authentication.
Comment 2 Mark Goodwin 2012-07-20 23:31:13 UTC 
Ken requested assignment, thanks.
Comment 3 Nathan Scott 2012-07-27 06:33:34 UTC 
Created attachment 600703 [details]
Resolve issues in decoding PCP instreq PDUs
Comment 4 Florian Weimer 2012-07-27 14:50:35 UTC 
(In reply to comment #3)
> Created attachment 600703 [details]
> Resolve issues in decoding PCP instreq PDUs

I think this is okay.  The expression

  sizeof(instance_req_t) - sizeof(pp->name) + namelen

looks a bit suspecious with regards to wraparound, but the namelen > pp->hdr.len before should cover that.
Comment 5 Florian Weimer 2012-08-09 14:12:25 UTC 
Integration testing revealed that src/pmcd/src/dopdus.c:DoInstance() does not check for the error return from __pmDecodeInstanceReq:

int
DoInstance(ClientInfo *cp, __pmPDU* pb)
{
    int			sts = 0, s;
    __pmTimeval		when;
    pmInDom		indom;
    int			inst;
    char		*name;
    __pmInResult		*inresult = NULL;
    AgentInfo		*ap;
    int			fdfail = -1;

    __pmDecodeInstanceReq(pb, &when, &indom, &inst, &name);
    if (when.tv_sec != 0 || when.tv_usec != 0) {

This can lead to crashes further down.
Comment 6 Nathan Scott 2012-08-09 23:00:02 UTC 
Created attachment 603365 [details]
Revised PCP instance request PDU decoding patch

Excellent catch, thanks!

I've also audited all of the other __pmDecode* call sites, to ensure they check their return codes.  There are a few __pmDecodeError calls which are currently safe, and the remainder do check.

cheers.
Comment 7 Huzaifa S. Sidhpurwala 2012-08-16 03:55:07 UTC 
Upstream patch:

http://oss.sgi.com/cgi-bin/gitweb.cgi?p=pcp/pcp.git;a=commit;h=f190942b552aa80d59bbe718866aa00b8e3fd5cc

This issue has been addressed in pcp-3.6.5
Comment 8 Huzaifa S. Sidhpurwala 2012-08-20 03:54:18 UTC 
This issue was addressed in Fedora and EPEL via the following security updates:

Fedora-16: https://admin.fedoraproject.org/updates/pcp-3.6.5-1.fc16
Fedora-17: https://admin.fedoraproject.org/updates/pcp-3.6.5-1.fc17
Rawhide:   https://admin.fedoraproject.org/updates/pcp-3.6.5-1.fc18

EPEL-5: https://admin.fedoraproject.org/updates/pcp-3.6.5-1.el5
EPEL-6: https://admin.fedoraproject.org/updates/pcp-3.6.5-1.el6