Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/crypto/openpgp: a couple of packages do not invoke .Close and hence leak io.Closer values in some return paths #53526

Closed
odeke-em opened this issue Jun 24, 2022 · 4 comments

Comments

@odeke-em
Copy link
Member

Internally at Orijtech Inc, while doing security audits and supply chain analyses for the Cosmos security team, we've found a bunch of leaking resources in a couple of packages

golang.org/x/crypto/openpgp/packet/compressed.go:98:3: leaking resource created on line 91
golang.org/x/crypto/openpgp/packet/literal.go:75:3: leaking resource created on line 68
golang.org/x/crypto/openpgp/packet/literal.go:79:3: leaking resource created on line 68
golang.org/x/crypto/openpgp/packet/literal.go:84:3: leaking resource created on line 68
golang.org/x/crypto/openpgp/packet/symmetrically_encrypted.go:268:3: leaking resource created on line 261
golang.org/x/crypto/openpgp/packet/symmetrically_encrypted.go:276:3: leaking resource created on line 261
golang.org/x/crypto/openpgp/packet/symmetrically_encrypted.go:281:3: leaking resource created on line 261
golang.org/x/crypto/openpgp/clearsign/clearsign.go:315:4: leaking resource created on line 300

and this problem exists due to the lack of finesse in defers and in using named error returns that can be used to close values.

/cc to my colleagues @willpoint @elias-orijtech @kirbyquerby

@gopherbot gopherbot added this to the Unreleased milestone Jun 24, 2022
@gopherbot
Copy link

Change https://go.dev/cl/413835 mentions this issue: openpgp/*: properly invoke .Close on errors

@dmitshur
Copy link
Contributor

Thanks for reporting. Please note #44226.

@cagedmantis
Copy link
Contributor

Closing since the package is frozen and deprecated.

@odeke-em
Copy link
Member Author

Thank you for chiming in @dmitshur and @cagedmantis! I wouldn't close this issue though as though are more of security fixes than bug fixes, or at least we can fly them as security fixes.

odeke-em added a commit to orijtech/crypto that referenced this issue Jun 28, 2022
Fixes resource leak bugs identified by Orijtech Cyber's
internal team and tooling.

Reported in golang/go#53526
@golang golang locked and limited conversation to collaborators Jun 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants