Bug 841314

Summary: pcp: malicious client can stop all pmcd server activity
Product: [Fedora] Fedora Reporter: Florian Weimer <fweimer>
Component: pcpAssignee: Mark Goodwin <mgoodwin>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 16CC: fche, 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 04:05:25 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, 841706    
Attachments:
Description Flags
Resolve an event-driven programming flaw in pmcd
none
Updated version of pmcd event-driven programming flaw patch none

Description Florian Weimer 2012-07-18 16:25:01 UTC
The pduread function in libpcp performs a select locally, waiting for more client data.  A malicious client can send individual bytes one by one, to avoid the timeout, and monopolize the pmcd server this way.  No authentication is required.

As a stop-gap measure, a low overall timeout for the whole PDU should be enforced, and not just a timeout for each individual recv call.  A real fix requires a proper event-driven style, that is, all I/O has to be performed in the main event loop, and the I/O routines have to be transformed to continuation-passing style.

Comment 2 Nathan Scott 2012-08-06 22:18:30 UTC
Created attachment 602593 [details]
Resolve an event-driven programming flaw in pmcd

Attached is Ken McDonells fix for this.  I was a little confused by 841706 which I think is another bug for the same issue, and attached it there late last week.

Here it is again anyway.

Comment 3 Florian Weimer 2012-08-07 08:00:20 UTC
(In reply to comment #2)
> Created attachment 602593 [details]
> Resolve an event-driven programming flaw in pmcd
> 
> Attached is Ken McDonells fix for this.

I think dead_hand.tv_usec needs to be adjusted in the if branch, so that it does not exceed 1000000.  The comparison needs to be >=, not >.

		    gettimeofday(&dead_hand, NULL);
		    dead_hand.tv_sec += wait.tv_sec;
		    dead_hand.tv_usec += wait.tv_usec;
		    if (dead_hand.tv_usec > 1000000)
			dead_hand.tv_sec++;

Perhaps a comment should be added that this is just a stop-gap measure and that the real fix is more involved.

Comment 4 Nathan Scott 2012-08-08 02:08:53 UTC
Created attachment 602904 [details]
Updated version of pmcd event-driven programming flaw patch

Incorporate Florian's review comments.

Comment 5 Huzaifa S. Sidhpurwala 2012-08-16 04:13:43 UTC
Upstream patch:

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

This issue has been addressed in pcp-3.6.5