Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  closed
 Priority:  Medium|  Milestone:  Tor:
  |  0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor:
  |  0.2.9.5-alpha
 Severity:  Normal| Resolution:  fixed
 Keywords:  028-backport review-group-13  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+
Changes (by nickm):

 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 Looks good; merging!

 FWIW, please remember that 0.2.9 fixups should go on maint-0.2.9

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor:
  |  0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor:
  |  0.2.9.5-alpha
 Severity:  Normal| Resolution:
 Keywords:  028-backport review-group-13  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+
Changes (by dgoulet):

 * status:  needs_revision => needs_review


Comment:

 Ok I took this one as 029 is near stable. See branch `bug20715_029_01`
 based on current release-0.2.9. This leak comes from 028 but I don't think
 I would backport it as this is not very security critical as far as I can
 tell.

 @neel: If you have more questions about this, please don't hesitate to
 ask. I understand that I did made an "aggressive" takeover of this ticket
 but know that we value very much your contribution so if we can answer
 more questions, we'll be happy to do so!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor:
  |  0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor:
  |  0.2.9.5-alpha
 Severity:  Normal| Resolution:
 Keywords:  028-backport review-group-13  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+

Comment (by nickm):

 The issue here is that you can't free "dest" in the way that you do, since
 signed_descriptor_t might not be a pointer to a chunk that was returned by
 tor_malloc().  It might be part of a routerinfo_t structure, for example.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor:
  |  0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor:
  |  0.2.9.5-alpha
 Severity:  Normal| Resolution:
 Keywords:  028-backport review-group-13  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+

Comment (by neel):

 I am new to contributing to Tor. Is it possible for you to give me more
 information on this so I can be able to track down the bug? (where to
 look, what functions, etc.)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor:
  |  0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor:
  |  0.2.9.5-alpha
 Severity:  Normal| Resolution:
 Keywords:  028-backport review-group-13  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * keywords:  028-backport => 028-backport review-group-13


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.5-alpha
 Severity:  Normal| Resolution:
 Keywords:  028-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  needs_review => needs_revision
 * keywords:   => 028-backport


Comment:

 Hi, neel!  That won't work actually, since the `dest` pointer there can be
 a pointer to the inside of another allocation.  (There are several
 structures that have signed_descriptor_t included in them.)

 Maybe we should just copy the part that clears the relevant parts of the
 dest structure?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.5-alpha
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  new => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:  Tor: 0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.5-alpha
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by neel):

 I'm not sure if the patch I submitted is everything necessary for this,
 but tell me what you think about this.

 Thanks,
 Neel Chauhan

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:  Tor: 0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.5-alpha
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by arma):

 signed_descriptor_move() looks interesting here:
 {{{
   ri = router_parse_entry_from_string(body,
 body+sd->signed_descriptor_len+sd->annotations_len,
  0, 1, NULL, NULL);
   if (!ri)
 return NULL;
   signed_descriptor_move(&ri->cache_info, sd);
 }}}

 So in the router_parse_entry(), we make a new cert and assign it to
 {{{
   router->cache_info.signing_key_cert = cert;
 }}}

 But then in signed_descriptor_move we
 {{{
   memcpy(dest, src, sizeof(signed_descriptor_t));
 }}}

 Does that clobber the old ri->cache_info.signing_key_cert with whatever
 was in sd?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20715 [Core Tor/Tor]: memory leak in tor_cert_parse()

[Tor Bug Tracker & Wiki]

#20715: memory leak in tor_cert_parse()
--+
 Reporter:  arma  |  Owner:
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:  Tor: 0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.5-alpha
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by arma):

 #19073 looks related but not obviously a culprit.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs