Bug 670802

Summary: curl --capath memory leakage
Product: Red Hat Enterprise Linux 6 Reporter: Kamil Dudka <kdudka>
Component: curlAssignee: Kamil Dudka <kdudka>
Status: CLOSED ERRATA QA Contact: BaseOS QE Security Team <qe-baseos-security>
Severity: high Docs Contact:
Priority: high    
Version: 6.0CC: emaldona, mhusnain, mvadkert, ovasik, prc, rcritten, rrelyea
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
libcurl leaked memory and eventually resulted in a failed NSS shutdown when more than one CA certificate was loaded. This is now fixed so that libcurl works as expected when more than one CA certificates is loaded.
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-05-19 13:12:34 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Attachments:
Description Flags
proposed fix rcritten: review+

Description Kamil Dudka 2011-01-19 12:51:54 UTC
Version-Release number of selected component (if applicable):
curl-7.19.7-19.el6

How reproducible:
100%

Steps to Reproduce:
$ cd `mktemp -d`
$ cp /etc/pki/tls/cert.pem cert1.pem
$ cp /etc/pki/tls/cert.pem cert2.pem
$ valgrind curl --capath . -so/dev/null https://bugzilla.redhat.com

Actual results:
LEAK SUMMARY:
   definitely lost: 156 bytes in 4 blocks
   indirectly lost: 0 bytes in 0 blocks
     possibly lost: 1,688,388 bytes in 6,710 blocks
   still reachable: 21,649 bytes in 104 blocks
        suppressed: 0 bytes in 0 blocks

Expected results:
LEAK SUMMARY:
   definitely lost: 836 bytes in 4 blocks
   indirectly lost: 208 bytes in 2 blocks
     possibly lost: 176 bytes in 4 blocks
   still reachable: 20,782 bytes in 99 blocks
        suppressed: 0 bytes in 0 blocks

Additional info:
It also prevent NSS from a proper shut down.  The bug has existed there forever.  The fix for bug #669702 only allowed it to cause more damage.

Comment 1 Kamil Dudka 2011-01-19 12:59:07 UTC
Created attachment 474252 [details]
proposed fix

Rob, could you please review the attached patch?

Comment 2 Kamil Dudka 2011-01-19 13:10:04 UTC
Comment on attachment 474252 [details]
proposed fix

>@@ -91,8 +92,12 @@ typedef struct {
>   PRInt32 version; /* protocol version valid for this cipher */
> } cipher_s;
> 
>-#define PK11_SETATTRS(x,id,v,l) (x)->type = (id); \
>-                     (x)->pValue=(v); (x)->ulValueLen = (l)
>+#define PK11_SETATTRS(_attr, _idx, _type, _val, _len) do {  \
>+  CK_ATTRIBUTE *ptr = (_attr) + ((_idx)++);                 \
>+  ptr->type = (_type);                                      \
>+  ptr->pValue = (_val);                                     \
>+  ptr->ulValueLen = (_len);                                 \
>+} while (0);

Oops, I left there an extra semicolon ^^^.  Please ignore it.  It's harmless anyway.

Comment 3 Rob Crittenden 2011-01-19 15:29:57 UTC
Comment on attachment 474252 [details]
proposed fix

Ok, right, loading more than 1 CA would lead to leaks and failed NSS shutdown. This looks good.

Comment 4 Kamil Dudka 2011-01-19 16:06:07 UTC
Thanks for review.

Comment 8 Kamil Dudka 2011-01-27 10:20:15 UTC
pushed upstream:

https://github.com/bagder/curl/commit/dc0a716

Comment 10 Misha H. Ali 2011-04-20 04:44:18 UTC
    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
libcurl leaked memory and eventually resulted in a failed NSS shutdown when more than one CA certificate was loaded. This is now fixed so that libcurl works as expected when more than one CA certificates is loaded.

Comment 11 errata-xmlrpc 2011-05-19 13:12:34 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0573.html