Opened 15 months ago

Closed 4 weeks ago

#19418 closed defect (fixed)

i2d_RSAPublicKey retval ignored in multiple callsites

Reported by: asn Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-bug-bounty, disaster-waiting-to-happen, review-group-22
Cc: Actual Points:
Parent ID: Points: 0.5
Reviewer: asn Sponsor: SponsorV-can

Description

Hello. Here follows a bug report by Guido Vranken received through the hackerone program.

There are various places in the codebase where we don't check the retval of i2d_RSA_PublicKey() (or its callers). That function can fail in cases of OOM, and this is something we should be handling correctly. This is a similar case to #17686.

Child Tickets

Change History (16)

comment:1 Changed 15 months ago by asn

Milestone: Tor: 0.2.9.x-finalTor: 0.2.???

comment:2 Changed 15 months ago by nickm

Keywords: 029-proposed removed
Milestone: Tor: 0.2.???Tor: 0.2.9.x-final

comment:3 Changed 14 months ago by isabela

Keywords: isaremoved added
Milestone: Tor: 0.2.9.x-finalTor: 0.2.???

comment:4 Changed 10 months ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:5 Changed 9 months ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:6 Changed 4 months ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:7 Changed 4 months ago by nickm

Keywords: isaremoved removed

comment:8 Changed 3 months ago by nickm

Keywords: disaster-waiting-to-happen added
Milestone: Tor: unspecifiedTor: 0.3.2.x-final

comment:9 Changed 2 months ago by nickm

Sponsor: SponsorV-can

comment:10 Changed 6 weeks ago by nickm

Status: newneeds_review

So it seems our major omission here has been checking the output of crypto_pk_get_digest. I have a patch for that in bug19418_029.

comment:11 Changed 6 weeks ago by nickm

Owner: set to nickm
Status: needs_reviewassigned

setting owner.

comment:12 Changed 6 weeks ago by nickm

Status: assignedneeds_review

comment:13 Changed 5 weeks ago by nickm

Keywords: review-group-22 added

comment:14 Changed 4 weeks ago by asn

Reviewer: asn

comment:15 Changed 4 weeks ago by asn

Status: needs_reviewmerge_ready

Looks good to me!

comment:16 Changed 4 weeks ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.1.x-final
Resolution: fixed
Status: merge_readyclosed

Merged to 0.3.1; not backporting.

Note: See TracTickets for help on using tickets.