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

Potentially replace unmaintained libraries ignored in audit.toml. #1203

Open
ruseinov opened this issue Jun 19, 2024 · 10 comments
Open

Potentially replace unmaintained libraries ignored in audit.toml. #1203

ruseinov opened this issue Jun 19, 2024 · 10 comments

Comments

@ruseinov
Copy link
Collaborator

ruseinov commented Jun 19, 2024

And revisit other ignores.

https://github.com/near/near-sdk-rs/blob/master/.cargo/audit.toml

@MPSxDev
Copy link

MPSxDev commented Jul 29, 2024

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I am a process engineer and I have a high problem-solving capacity. I am currently co-founder and developer of my own startup.

How I plan on tackling this issue

My work can be divided into 3 stages:

  1. Research new solutions from other updated libraries
  2. Deploy locally to see its usage and avoid errors or problems.
  3. Implement until the desired goal is achieved.

This is my first ODack I hope to contribute a lot, I am available to start immediately

@frol
Copy link
Collaborator

frol commented Jul 29, 2024

@MPSxDev Go for it!

@MPSxDev
Copy link

MPSxDev commented Jul 29, 2024

Research Outcome:

This is the research work carried out, first checking that the repositories are maintained and active, and then verifying the existence of documentation in docs.rs. Below I leave the points found in the audit.toml file

Atty: This is no longer maintained 2 years ago an alternative:

  • Use std.io.IsTerminal, introduced in Rust 1.70.0 which is part of The Rust Standard Library. Reference docs.rs

Wee_alloc: This has not been maintained for 3 years, a solution may be to implement

curve25519-dalek: Because it has vulnerabilities I found this alternative:

I look forward to your comments and any other requests that may be required.

@frol
Copy link
Collaborator

frol commented Jul 30, 2024

We had a prior discussion about wee_alloc and decided to keep it: #1151

atty, curve25519-dalek - they are not direct dependencies, right? Can we update any of direct dependencies to fix it?

@MPSxDev
Copy link

MPSxDev commented Jul 30, 2024

  • In the case of atty, they themselves recommend using std.io.IsTerminal since it was incorporated into the Rust Standard Library. Ref IMG

image

  • curve25519-dalek appears to have vulnerabilities but using Scalar29::sub (32-bit) and Scalar52::sub which can be solved by adding a volatile read as an optimization barrier as stated in this report: RUSTEC Report

@frol
Copy link
Collaborator

frol commented Jul 31, 2024

@MPSxDev near-sdk-rs does not directly depend on atty. Please, inspect the dependencies tree to identify which crates we need to update in order to upgrade/eliminate atty and curve25519-dalek. Once identified, we could make a decision whether we can contribute the fix to those dependencies as there is nothing near-sdk-rs can do about it directly.

@MPSxDev
Copy link

MPSxDev commented Aug 5, 2024

After a thorough investigation of the packages that include the involved dependencies.

These include the atty dependency:

  • cargo-near 0.5.2
  • env_logger 0.9.3

These include ed25519-dalek ( include curve25519-dalek):

  • near-crypto 0.23.0 and 0.20.1
  • near-cli-rs 0.7.8
  • near-vm-runner 0.20.1 and 0.23.0
  • slip10 0.4.3

Below are the affected packages used, including the current version and whether they continue to use these affected dependencies.

  • cargo-near: The current version is 0.6.4, last updated on 2024-07-22. It could migrate atty to std.io.IsTerminal, as recommended by atty.
  • env_logger: There is currently a new version 0.11.5 that does not use atty; it should be updated to this new version.
  • near-crypto: The current version is 0.23.0 but it is using the affected version of curve25519-dalek, which should be updated to version >=4.1.3. It is also using version 2.1.0 of ed25519-dalek.
  • near-cli-rs: The current version is 0.13.0 and uses version ed25519-dalek v2.
  • near-vm-runner: The current version is 0.23.0 and continues to use version 2.1.0 of ed25519-dalek.
  • slip10: This package is 3 years old and supports ed25519-dalek v1.
  • ed25519-dalek: The current version is v2.1.1 and should be updated to use curve25519-dalek v4.1.3, which fixes the vulnerability according to this Reference.

In summary, the following steps should be taken:

  1. Migrate from atty to std.io.IsTerminal in cargo-near.
  2. Update env_logger to the latest version.
  3. For near-crypto, near-cli-rs, and near-vm-runner, use versions higher than v2 for ed25519-dalek, which according to rustec.org includes the patch for the vulnerability in versions >=2.
  4. For all mentioned in point 4, since ed25519-dalek uses curve25519-dalek as a dependency, version >=4.1.3 should be used.

@frol
Copy link
Collaborator

frol commented Aug 5, 2024

@MPSxDev Great summary! We definitely can act on the first two steps immediately. slip10 has been replaced with slipped10, and that should have been subsequently updated in the recent near-cli-rs and cargo-near releases. Can you give it a try upgrading the dependencies and authoring the atty PR to cargo-near as part of this issue?

@MPSxDev
Copy link

MPSxDev commented Aug 5, 2024

@frol Thank you very much. Yes, I can try to update this. If I have any questions, I will let you know.

frol pushed a commit to near/cargo-near that referenced this issue Aug 6, 2024
**Atty is replaced by std::io::IsTerminal**

Because atty has been unmaintained for 4 years, it is necessary to
update it in this case to std::io::IsTerminal, which is part of the
_Rust Standard Library_.
[Ref](https://doc.rust-lang.org/std/io/trait.IsTerminal.html)

_Changes:_

- File: cargo-near\src\commands\build_command\docker.rs
Description: Atty is replaced by std::io::IsTerminal

- File: cargo-near\src\common.rs
Description: Atty is replaced by std::io::IsTerminal

- File: cargo-near\cargo-near\src\main.rs
Description: Atty is replaced by std::io::IsTerminal

- File: cargo-near\Cargo.toml
Description: Atty is removed and env_logger is updated to 0.11.5 because
the previous version used atty.

- File: cargo-near\integration-tests\Cargo.toml
Description: env_logger is updated to 0.11.5 because the previous
version used atty.

Reference issue: [Potentially replace unmaintained libraries
ignored](near/near-sdk-rs#1203)
@MPSxDev
Copy link

MPSxDev commented Aug 21, 2024

This issue should be closed, I have already done the audit on the latest version and there is no presence of atty and the versions of other dependencies are already updated to versions without the vulnerability. @frol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants