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

telemetry: replace the uuid package dependency #3715

Merged
merged 7 commits into from
Mar 4, 2022

Conversation

tsachiherman
Copy link
Contributor

@tsachiherman tsachiherman commented Mar 4, 2022

Summary

The satori/go.uuid dependency was reported to contain a vulnerability ( https://nvd.nist.gov/vuln/detail/CVE-2021-3538#VulnChangeHistorySection ).

While this vulnerability would have no negative impact on the Algorand blockchain, it does mislead automated vulnerability detectors and points the Algorand codebase to be considered insecure.

Given that this package was used in so few locations, and in such a small scope, I have removed the usage of it completely. Instead, I have implemented a random UUID which would work correctly for our needs.

Test Plan

Unit tests added.

@codecov-commenter
Copy link

codecov-commenter commented Mar 4, 2022

Codecov Report

Merging #3715 (237470b) into master (12c2047) will increase coverage by 0.00%.
The diff coverage is 78.57%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3715   +/-   ##
=======================================
  Coverage   49.55%   49.56%           
=======================================
  Files         391      392    +1     
  Lines       68550    68561   +11     
=======================================
+ Hits        33973    33981    +8     
- Misses      30840    30844    +4     
+ Partials     3737     3736    -1     
Impacted Files Coverage Δ
node/node.go 23.33% <0.00%> (ø)
util/uuid/uuid.go 81.81% <81.81%> (ø)
logging/telemetry.go 81.35% <100.00%> (ø)
logging/telemetryConfig.go 81.81% <100.00%> (ø)
ledger/blockqueue.go 82.18% <0.00%> (-2.88%) ⬇️
network/wsPeer.go 65.83% <0.00%> (-2.23%) ⬇️
catchup/peerSelector.go 98.95% <0.00%> (-1.05%) ⬇️
cmd/tealdbg/debugger.go 71.42% <0.00%> (-0.99%) ⬇️
network/wsNetwork.go 62.99% <0.00%> (ø)
ledger/acctupdates.go 69.00% <0.00%> (+0.66%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12c2047...237470b. Read the comment docs.

@tsachiherman tsachiherman marked this pull request as ready for review March 4, 2022 01:41
winder
winder previously approved these changes Mar 4, 2022
Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

gmalouf
gmalouf previously approved these changes Mar 4, 2022
util/uuid/uuid.go Outdated Show resolved Hide resolved
@tsachiherman tsachiherman dismissed stale reviews from gmalouf and winder via 7157deb March 4, 2022 03:25
@tsachiherman tsachiherman requested a review from cce March 4, 2022 14:41
@tsachiherman tsachiherman merged commit ac04ef9 into algorand:master Mar 4, 2022
@tsachiherman tsachiherman deleted the tsachi/removeuuid branch March 4, 2022 18:03
@egieseke egieseke mentioned this pull request Mar 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants