ipinfo / rust

IPinfo Rust library for IPinfo API (IP geolocation and other types of IP data)
https://ipinfo.io
Apache License 2.0
56 stars 14 forks source link

Async support (and mitigate CVEs) #32

Closed adamchalmers closed 1 year ago

adamchalmers commented 1 year ago

Hi! Thanks very much for making this crate. cargo audit indicates that it pulls in several dependencies which have CVEs:

Crate:     hyper
Version:   0.12.36
Title:     Integer overflow in `hyper`'s parsing of the `Transfer-Encoding` header leads to data loss
Date:      2021-07-07
ID:        RUSTSEC-2021-0079
URL:       https://rustsec.org/advisories/RUSTSEC-2021-0079
Solution:  Upgrade to >=0.14.10
Dependency tree:
hyper 0.12.36
├── reqwest 0.9.24
│   └── ipinfo 1.0.0
└── hyper-tls 0.3.2
    └── reqwest 0.9.24

Crate:     hyper
Version:   0.12.36
Title:     Lenient `hyper` header parsing of `Content-Length` could allow request smuggling
Date:      2021-07-07
ID:        RUSTSEC-2021-0078
URL:       https://rustsec.org/advisories/RUSTSEC-2021-0078
Solution:  Upgrade to >=0.14.10

Crate:     lru
Version:   0.6.6
Title:     Use after free in lru crate
Date:      2021-12-21
ID:        RUSTSEC-2021-0130
URL:       https://rustsec.org/advisories/RUSTSEC-2021-0130
Solution:  Upgrade to >=0.7.1
Dependency tree:
lru 0.6.6
└── ipinfo 1.0.0

Crate:     time
Version:   0.1.45
Title:     Potential segfault in the time crate
Date:      2020-11-18
ID:        RUSTSEC-2020-0071
URL:       https://rustsec.org/advisories/RUSTSEC-2020-0071
Solution:  Upgrade to >=0.2.23
Dependency tree:
time 0.1.45
├── reqwest 0.9.24
│   └── ipinfo 1.0.0
├── hyper 0.12.36
│   ├── reqwest 0.9.24
│   └── hyper-tls 0.3.2
│       └── reqwest 0.9.24
├── cookie_store 0.7.0
│   └── reqwest 0.9.24
└── cookie 0.12.0
    ├── reqwest 0.9.24
    └── cookie_store 0.7.0

Crate:     tokio
Version:   0.1.22
Title:     Data race when sending and receiving after closing a `oneshot` channel
Date:      2021-11-16
ID:        RUSTSEC-2021-0124
URL:       https://rustsec.org/advisories/RUSTSEC-2021-0124
Solution:  Upgrade to >=1.8.4, <1.9.0 OR >=1.13.1
Dependency tree:
tokio 0.1.22
├── reqwest 0.9.24
│   └── ipinfo 1.0.0
└── hyper 0.12.36
    ├── reqwest 0.9.24
    └── hyper-tls 0.3.2
        └── reqwest 0.9.24

Crate:     failure
Version:   0.1.8
Warning:   unmaintained
Title:     failure is officially deprecated/unmaintained
Date:      2020-05-02
ID:        RUSTSEC-2020-0036
URL:       https://rustsec.org/advisories/RUSTSEC-2020-0036
Dependency tree:
failure 0.1.8
└── cookie_store 0.7.0
    └── reqwest 0.9.24
        └── ipinfo 1.0.0

Crate:     net2
Version:   0.2.38
Warning:   unmaintained
Title:     `net2` crate has been deprecated; use `socket2` instead
Date:      2020-05-01
ID:        RUSTSEC-2020-0016
URL:       https://rustsec.org/advisories/RUSTSEC-2020-0016
Dependency tree:
net2 0.2.38
├── miow 0.2.2
│   └── mio 0.6.23
│       ├── tokio-tcp 0.1.4
│       │   ├── tokio 0.1.22
│       │   │   ├── reqwest 0.9.24
│       │   │   │   └── ipinfo 1.0.0
│       │   │   └── hyper 0.12.36
│       │   │       ├── reqwest 0.9.24
│       │   │       └── hyper-tls 0.3.2
│       │   │           └── reqwest 0.9.24
│       │   └── hyper 0.12.36
│       ├── tokio-reactor 0.1.12
│       │   ├── tokio-tcp 0.1.4
│       │   ├── tokio 0.1.22
│       │   └── hyper 0.12.36
│       └── tokio 0.1.22
├── mio 0.6.23
└── hyper 0.12.36

error: 5 vulnerabilities found!
warning: 2 allowed warnings found

I've fixed this by bumping the crate versions. All in all, it's a very small PR. If you merge it, could you please also publish a new version of the crate, so that I can pull it into my main project? Until then I'll use a fork. No real rush, but I'm sure the broad Rust community would appreciate it :) Thanks very much and have a good weekend!

rm-Umar commented 1 year ago

@adamchalmers Thank you for contributing but the tests are failing so we can't merge the PR. @fayzanx can you have a look at it?

rm-Umar commented 1 year ago

Ran the tests locally and everything is running fine. The tests are failing because the github action is running in @adamchalmers repo that doesn't have a token and the token is required to do batch lookup. @fayzanx will let you have a final look before merging. Waiting for your approval. @adamchalmers thanks again for highlighting the issue and contributing 🙏🏽 I'll release the new crate as soon as we merge this and will let you know.

adamchalmers commented 1 year ago

Thanks! By the way, I'm sorry for all the formatting changes, I forgot to disable format-on-save in my IDE. I've amended the PR to remove the formatting changes.

Alternatively, y'all could copy these changes into a branch in the ipinfo/rust project, then CI could test my changes while pulling your tokens etc to actually pass the tests :)

adamchalmers commented 1 year ago

OK, bumping reqwest to 0.11 breaks existing users who were using it an async context with block_on. It panics with Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context.

The only way to properly fix this was to add async support, so, here's a PR to do that! It's very straightforward.

rm-Umar commented 1 year ago

@adamchalmers thanks again for contributing. I've released the latest crate 1.1.0 you can use that now.

adamchalmers commented 1 year ago

Thanks! I think this is a breaking change as it changes a method to async which was not before. So shouldn't it be v2.0 not v1.1?

rm-Umar commented 1 year ago

@adamchalmers thanks for pointing it out, totally my bad 🙏🏽 . I've yanked the 1.1.0 updated examples and releasing 2.0.0

rm-Umar commented 1 year ago

@adamchalmers thanks for saving the day again, I've released the 2.0.0

adamchalmers commented 1 year ago

Happy to help. Thanks for responding to all my concerns so quickly!