Bug 841240 - __pmDecodeInstanceReq heap buffer overflow
__pmDecodeInstanceReq heap buffer overflow
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: pcp (Show other bugs)
16
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Ken McDonell
Fedora Extras Quality Assurance
: Security
Depends On:
Blocks: 840765 CVE-2012-3418
  Show dependency treegraph
 
Reported: 2012-07-18 09:44 EDT by Florian Weimer
Modified: 2012-08-19 23:54 EDT (History)
4 users (show)

See Also:
Fixed In Version: pcp-3.6.5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-08-19 23:54:18 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Resolve issues in decoding PCP instreq PDUs (1.48 KB, patch)
2012-07-27 02:33 EDT, Nathan Scott
no flags Details | Diff
Revised PCP instance request PDU decoding patch (2.64 KB, patch)
2012-08-09 19:00 EDT, Nathan Scott
no flags Details | Diff

  None (edit)
Description Florian Weimer 2012-07-18 09:44:32 EDT
__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 19:31:13 EDT
Ken requested assignment, thanks.
Comment 3 Nathan Scott 2012-07-27 02:33:34 EDT
Created attachment 600703 [details]
Resolve issues in decoding PCP instreq PDUs
Comment 4 Florian Weimer 2012-07-27 10:50:35 EDT
(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 10:12:25 EDT
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 19:00:02 EDT
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-15 23:55:07 EDT
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

Note You need to log in before you can comment on or make changes to this bug.