Bug 841240 - __pmDecodeInstanceReq heap buffer overflow
Summary: __pmDecodeInstanceReq heap buffer overflow
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: pcp
Version: 16
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Ken McDonell
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 840765 CVE-2012-3418
TreeView+ depends on / blocked
 
Reported: 2012-07-18 13:44 UTC by Florian Weimer
Modified: 2012-08-20 03:54 UTC (History)
4 users (show)

Fixed In Version: pcp-3.6.5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-08-20 03:54:18 UTC
Type: Bug
Embargoed:


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

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


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