Skip to content

Conversation

@AlexsandroRyan
Copy link
Contributor

@AlexsandroRyan AlexsandroRyan commented Aug 15, 2024

This is just to address the CVE-2024-34478 reported by Checkmarx tool when run in a repository that uses btcutil.

There's more information about the CVE here https://www.cve.org/CVERecord?id=CVE-2024-34478

Fixes #2236

@AlexsandroRyan AlexsandroRyan mentioned this pull request Aug 15, 2024
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The btcutil submodule only requires certain functions of btcd as a library. So the affected CVE code isn't active here.
But since the CVE reporting tools don't know this, it's good to get rid of any warnings. So thanks for the fix.

@AlexsandroRyan
Copy link
Contributor Author

@guggero I think a go mod tidy was missing... I believe this might fix the CI.

@coveralls
Copy link

coveralls commented Aug 15, 2024

Pull Request Test Coverage Report for Build 10405282717

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.008%) to 57.23%

Files with Coverage Reduction New Missed Lines %
peer/peer.go 6 74.16%
Totals Coverage Status
Change from base Build 10312227216: -0.008%
Covered Lines: 29838
Relevant Lines: 52137

💛 - Coveralls

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. Thanks for fixing that. Can you please squash the two commits into one?

@AlexsandroRyan AlexsandroRyan force-pushed the pkg-update-checkmarx-cve branch from 969a168 to 913f95b Compare August 15, 2024 14:19
@AlexsandroRyan
Copy link
Contributor Author

@guggero Done 👍

@AlexsandroRyan
Copy link
Contributor Author

AlexsandroRyan commented Aug 15, 2024

@guggero Do you know if there is any update on when this change might be included in a new official release of btcutil?

@guggero
Copy link
Collaborator

guggero commented Aug 15, 2024

I can push a new tag after merging this PR. There aren't actual "releases" for btcutil. But a new tag should notify dependency management tools.

@guggero guggero linked an issue Aug 15, 2024 that may be closed by this pull request
@Roasbeef Roasbeef merged commit bda7977 into btcsuite:master Aug 15, 2024
@guggero
Copy link
Collaborator

guggero commented Aug 15, 2024

Pushed a new tag: btcutil/v1.1.6

@AlexsandroRyan
Copy link
Contributor Author

Pushed a new tag: btcutil/v1.1.6

Thanks @guggero!

melekes added a commit to cometbft/cometbft that referenced this pull request Aug 20, 2024
Author: [AlexsandroRyan](https://github.com/AlexsandroRyan)

This pull request addresses all CVEs reported by the Checkmarx tool
during its execution on this repository.

The previous discussion can be found here:
https://github.com/cometbft/cometbft/discussions/3558.

I have updated all necessary dependencies to fix the identified CVEs,
but some vulnerabilities remain unresolved. I would appreciate any
assistance in addressing these remaining issues.

1. CVE-2021-3538: This issue is related to the github.com/satori/go.uuid
package, which is a dependency of
[tm-load-test](https://github.com/informalsystems/tm-load-test). We have
already submitted a PR to address this:
informalsystems/tm-load-test#221.

2. CVE-2024-24786: This vulnerability pertains to the
google.golang.org/protobuf package. Running go mod graph | grep
google.golang.org/protobuf reveals that many packages are using the
vulnerable version. It’s unclear if updating them individually is
feasible.

3. CVE-2024-34478: This vulnerability is associated with
github.com/btcsuite/btcd, a dependency of
github.com/btcsuite/btcd/btcutil, which is currently used at a version
lower than 0.24.0. We have also submitted a pull request for this:
btcsuite/btcd#2235.

Please let us know if this approach is sufficient or if there is a more
efficient way to resolve these issues.

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

#### PR checklist

- [ ] ~Tests written/updated~
- [ ] ~Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)~
- [ ] ~Updated relevant documentation (`docs/` or `spec/`) and code
comments~
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Alexsandro <alexsandrocosta855@gmail.com>
Co-authored-by: Alessandro <alessandro@informal.systems>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CVE-2024-34478

4 participants