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

Compilation error with url 2.5.3 / idna 1.0.3 with Rust < 1.81 and edition 2021 (resolver 2) when sqlx-core is in the dependency graph #992

Open
1 task done
TotalKrill opened this issue Nov 4, 2024 · 30 comments

Comments

@TotalKrill
Copy link

TotalKrill commented Nov 4, 2024

Edited by @hsivonen to put resolution options at the top for folks who come here:

This compilation error is due to a combination of:

  1. SQLx unnecessarily turning off the default features of url, and from a proc macro context specifically. (Request for SQLx not to do that: Please do not turn off default features for url launchbadge/sqlx#3589)
  2. url 2.5.3 (and idna 1.0.3) conditionally raising MSRV to 1.81 when default features are off. (From no_std support for the url crate #831)
  3. resolver = "2", which is implied by edition = "2021" performing feature resolution for proc macro dependencies separately from normal dependencies, so a normal dependency on url with default features enabled does not cause default features to be enabled for the proc macro case.

The most obvious way to resolve this compilation error is to upgrade Rust to 1.81 or later. (If you are using SQLx 0.8.x, your Rust version has to already be 1.79 or later, so it's not a big jump.)

Alternative resolution without upgrading Rust:

  1. Put resolver = "1" in the [workspace] section of your top-level Cargo.toml. If there's no such section yet, create a [workspace] section at the top of your top-level Cargo.toml.
  2. If you don't already have a non-proc macro dependency on url with default features enabled somewhere in your dependency graph, add url = "2.5.3" to the [dependencies] section of one of your crates (the top-level is the most obvious place, but it does not have to be on the top level).

(Conditionally raising MSRV to 1.81 is not a semver break, because raising MSRV isn't a semver break.)

Original report follows:

  • Note that this crate implements the URL Standard not RFC 1738 or RFC 3986

Describe the bug

After update to v1.0.3 wich is picked up automatically since its a patch upgrade, our code no longer compiles the same.

rust versions tested:

  • rustc 1.80.1
  • rustc 1.81.0
   Compiling idna v1.0.3
error[E0658]: use of unstable library feature 'error_in_core'
  --> /home/krille/.cargo/registry/src/index.crates.io-6f17d22bba15001f/idna-1.0.3/src/lib.rs:78:6
   |
78 | impl core::error::Error for Errors {}
   |      ^^^^^^^^^^^^^^^^^^
   |
   = note: see issue #103765 <https://github.com/rust-lang/rust/issues/103765> for more information
@TotalKrill TotalKrill changed the title Compilation error with update to 1.0.3 Compilation error with update to idna 1.0.3 Nov 4, 2024
@valenting
Copy link
Collaborator

@TotalKrill
Is your crate depending on idna directly with default-features = false.
Or is the issue url failing to propagate the std feature to idna?

@TotalKrill
Copy link
Author

We are depending on url, not idna directly

@TotalKrill
Copy link
Author

@valenting seems i was wrong about 1.81.0, seems to work there...

@hsivonen
Copy link
Collaborator

hsivonen commented Nov 4, 2024

Are you depending on url with default-features = false?

I tried to repro on 1.80.1, and the following compiles:

rustup default 1.80
cargo new --bin testurl
cd testurl
cargo add url
cargo build

@TotalKrill
Copy link
Author

we have a workspace, but we only depend on url like this: url = "2.5.0"

@TotalKrill
Copy link
Author

TotalKrill commented Nov 4, 2024

running cargo tree -i url -f "{p} {f}"

this is what our tree ends up including, with two different versions of URL included, probably by one of our other deps.

  • url v2.5.3 default,serde,std
  • url v2.5.3

Edit: I meant version 2.5.3, 2.5.0 is the version that compiles

@hsivonen
Copy link
Collaborator

hsivonen commented Nov 4, 2024

url 2.5.0 and url 2.5.2 depended on idna 0.5. url 2.5.1 depended on idna 1.0.

The introduction of no_std support to url in 2.5.3 and the introduction of core::error::Error to idna in 1.0.3 was intended not to change MSRV to 1.81 with default features but was expected to change MSRV to 1.81 for default-features = false users.

Do you somehow have url 2.5.1 in your dependency graph? Can you force Cargo to take url 2.5.3 in its place?

@TotalKrill
Copy link
Author

Oh sorry, I am currently forcing our software to take 2.5.0, since 2.5.3 does not compile. Updated the previous comment to show that the cargo tree command showed 2.5.3

@hsivonen
Copy link
Collaborator

hsivonen commented Nov 4, 2024

So if you have url 2.5.3 without features and url 2.5.3 with the feature std, Cargo should unify those to one copy of url with the feature std. Are you sure the edited comment about cargo tree is correct?

Aside: This isn't specific to idna. If I use Rust 1.80.1 with:

[dependencies]
url = { version = "2.5.3", default-features = false }
idna = "1"

...idna compiles and the compilation of url fails with a complaint about impl core::error::Error for ParseError {} in url.

Twixes added a commit to PostHog/posthog that referenced this issue Nov 4, 2024
Twixes added a commit to PostHog/posthog that referenced this issue Nov 4, 2024
@TotalKrill
Copy link
Author

yep, my edited comment stands. But with what you say, something else might be changing to default-features = false

@hsivonen
Copy link
Collaborator

hsivonen commented Nov 4, 2024

As long as something in your dependency graph depends on url without setting default-features = false, something else setting default-features = false shouldn't matter.

@Twixes, GitHub autolinks a couple of your changesets referencing this issue. Do you have minimized steps to reproduce?

@valenting
Copy link
Collaborator

I think cargo tree -e features might explain if there's such a dependency conflict.

@mental32
Copy link

mental32 commented Nov 4, 2024

Just felt this through my direct dependency to sqlx. I'm commenting here but I'm not sure who, sqlx or idna, is responsible in this case. (@hsivonen would appreciate your take here)

minimal verifiable example:

cd /tmp
cargo new idna-992-repro
cd idna-992-repro/
cargo add sqlx
cargo check

And I get the same compilation error as @TotalKrill

The latest version of SQLx is v0.8.2 in the example, but in my original encounter its v0.7.4.

(for the benefit of solution-seekers pinning to url = "=2.5.2" is the fix i am using for now.)

cargo --version --verbose
cargo 1.80.1 (376290515 2024-07-16)
release: 1.80.1
commit-hash: 37629051518c3df9ac2c1744589362a02ecafa99
commit-date: 2024-07-16
host: aarch64-apple-darwin
libgit2: 1.7.2 (sys:0.18.3 vendored)
libcurl: 8.7.1 (sys:0.4.72+curl-8.6.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Mac OS 15.0.1 [64-bit]
rustc --version --verbose
rustc 1.80.1 (3f5fd8dd4 2024-08-06)
binary: rustc
commit-hash: 3f5fd8dd41153bc5fdca9427e9e05be2c767ba23
commit-date: 2024-08-06
host: aarch64-apple-darwin
release: 1.80.1
LLVM version: 18.1.7

@hsivonen
Copy link
Collaborator

hsivonen commented Nov 4, 2024

As long as something in your dependency graph depends on url without setting default-features = false, something else setting default-features = false shouldn't matter.

Interestingly, this turns out not to be true.

rustup default 1.80
cargo new idna-992-repro
cd idna-992-repro/
cargo add sqlx
cargo add url
cargo check

Still fails. (Note the added cargo add url.)

@domenukk why does sqlx-core's dependency on url with default-features = false not unify with the top-level's dependency on url (with default features)?

@TotalKrill
Copy link
Author

We are also using sqlx, so might be through that

@hsivonen
Copy link
Collaborator

hsivonen commented Nov 4, 2024

sqlx-core has url = { version = "2.2.2", default-features = false }. How has that not failed due to url requiring alloc before?

@Twixes
Copy link

Twixes commented Nov 4, 2024

@hsivonen I pinned idna to 0.5 in Cargo.toml's [dependencies] – seems to solve the issue. It also appears to be stemming from sqlx for us

@hsivonen hsivonen changed the title Compilation error with update to idna 1.0.3 Compilation error with url 2.5.3 / idna 1.0.3 when sqlx-core is in the dependency graph Nov 4, 2024
@hsivonen
Copy link
Collaborator

hsivonen commented Nov 4, 2024

@hsivonen I pinned idna to 0.5 in Cargo.toml's [dependencies] – seems to solve the issue.

Do you mean PostHog/posthog@5f6e5d1 fixed it for you? That's really weird. By what mechanism should that help?

@Twixes
Copy link

Twixes commented Nov 4, 2024

Actually you are right, the Cargo.toml change doesn't help. It's the --locked arg of cargo install that's the fix.

For us, the issue was with this command:
cargo install [email protected] --no-default-features --features native-tls,postgres
Used to work with Rust 1.77, but starting today it only works on 1.81+ – because of idna 1.0 getting installed. Our lockfile has already been specifying idna 0.5 though, now that I look at it.

By appending --locked to cargo install [email protected] --no-default-features --features native-tls,postgres we make it so that the lockfile is respected, fixing installation. (It looks like there's no alternative way to restrict transitive dependencies in cargo install)

@hsivonen hsivonen changed the title Compilation error with url 2.5.3 / idna 1.0.3 when sqlx-core is in the dependency graph Compilation error with url 2.5.3 / idna 1.0.3 with Rust < 1.81 when sqlx-core is in the dependency graph Nov 4, 2024
@hsivonen
Copy link
Collaborator

hsivonen commented Nov 4, 2024

The sqlx change that added default-features = false was launchbadge/sqlx@d76b135 which doesn't explain why.

This was referenced Dec 11, 2024
github-merge-queue bot pushed a commit to hyperlane-xyz/hyperlane-monorepo that referenced this issue Dec 17, 2024
### Description

Updated `sea-orm` and `sea-orm-migration` to `1.1.1` (latest as of
opening this PR) and resolved any issues that occurred due to the
version bump.

### Drive-by changes

- Removed `sea-orm` from `rust/sealevel`
- Updated `sea-orm-migration` data types
- Bumped up the `rust-toolchain` to `1.81.0` due to `sea-orm-cli` issue:
servo/rust-url#992

### Related issues

- Resolves #4793

### Backward compatibility

Yes

### Testing

Manual, as described in #4793
taiki-e added a commit to openrr/openrr that referenced this issue Dec 20, 2024
```
error[vulnerability]: `idna` accepts Punycode labels that do not produce any non-ASCII when decoded
    ┌─ /home/runner/work/openrr/openrr/Cargo.lock:260:1
    │
260 │ idna 0.1.5 registry+https://github.com/rust-lang/crates.io-index
    │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ security vulnerability detected
    │
    ├ ID: RUSTSEC-2024-0421
    ├ Advisory: https://rustsec.org/advisories/RUSTSEC-2024-0421
    ├ `idna` 0.5.0 and earlier accepts Punycode labels that do not produce any non-ASCII output, which means that either ASCII labels or the empty root label can be masked such that they appear unequal without IDNA processing or when processed with a different implementation and equal when processed with `idna` 0.5.0 or earlier.
      
      Concretely, `example.org` and `xn--example-.org` become equal after processing by `idna` 0.5.0 or earlier. Also, `example.org.xn--` and `example.org.` become equal after processing by `idna` 0.5.0 or earlier.
      
      In applications using `idna` (but not in `idna` itself) this may be able to lead to privilege escalation when host name comparison is part of a privilege check and the behavior is combined with a client that resolves domains with such labels instead of treating them as errors that preclude DNS resolution / URL fetching and with the attacker managing to introduce a DNS entry (and TLS certificate) for an `xn--`-masked name that turns into the name of the target when processed by `idna` 0.5.0 or earlier.
      
      ## Remedy
      
      Upgrade to `idna` 1.0.3 or later, if depending on `idna` directly, or to `url` 2.5.4 or later, if depending on `idna` via `url`. (This issue was fixed in `idna` 1.0.0, but versions earlier than 1.0.3 are not recommended for other reasons.)
      
      When upgrading, please take a moment to read about [alternative Unicode back ends for `idna`](https://docs.rs/crate/idna_adapter/latest).
      
      If you are using Rust earlier than 1.81 in combination with SQLx 0.8.2 or earlier, please also read an [issue](servo/rust-url#992) about combining them with `url` 2.5.4 and `idna` 1.0.3.
      
      ## Additional information
      
      This issue resulted from `idna` 0.5.0 and earlier implementing the UTS 46 specification literally on this point and the specification having this bug. The specification bug has been fixed in [revision 33 of UTS 46](https://www.unicode.org/reports/tr46/tr46-33.html#Modifications).
      
      ## Acknowledgements
      
      Thanks to kageshiron for recognizing the security implications of this behavior.
    ├ Announcement: https://bugzilla.mozilla.org/show_bug.cgi?id=1887898
    ├ Solution: Upgrade to >=1.0.0 (try `cargo update -p idna`)
    ├ idna v0.1.5
      └── url v1.7.2
          └── hyper v0.10.16
              └── xml-rpc v0.0.12
                  └── rosrust v0.9.11
                      ├── arci-ros v0.1.0
                      │   ├── arci-ros-hygiene-test v0.0.0
                      │   └── openrr-apps v0.1.0
                      │       └── openrr v0.1.0
                      └── tf_rosrust v0.1.0
                          └── arci-ros v0.1.0 (*)
```
taiki-e added a commit to openrr/openrr that referenced this issue Dec 20, 2024
```
error[vulnerability]: `idna` accepts Punycode labels that do not produce any non-ASCII when decoded
    ┌─ /home/runner/work/openrr/openrr/Cargo.lock:260:1
    │
260 │ idna 0.1.5 registry+https://github.com/rust-lang/crates.io-index
    │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ security vulnerability detected
    │
    ├ ID: RUSTSEC-2024-0421
    ├ Advisory: https://rustsec.org/advisories/RUSTSEC-2024-0421
    ├ `idna` 0.5.0 and earlier accepts Punycode labels that do not produce any non-ASCII output, which means that either ASCII labels or the empty root label can be masked such that they appear unequal without IDNA processing or when processed with a different implementation and equal when processed with `idna` 0.5.0 or earlier.

      Concretely, `example.org` and `xn--example-.org` become equal after processing by `idna` 0.5.0 or earlier. Also, `example.org.xn--` and `example.org.` become equal after processing by `idna` 0.5.0 or earlier.

      In applications using `idna` (but not in `idna` itself) this may be able to lead to privilege escalation when host name comparison is part of a privilege check and the behavior is combined with a client that resolves domains with such labels instead of treating them as errors that preclude DNS resolution / URL fetching and with the attacker managing to introduce a DNS entry (and TLS certificate) for an `xn--`-masked name that turns into the name of the target when processed by `idna` 0.5.0 or earlier.

      ## Remedy

      Upgrade to `idna` 1.0.3 or later, if depending on `idna` directly, or to `url` 2.5.4 or later, if depending on `idna` via `url`. (This issue was fixed in `idna` 1.0.0, but versions earlier than 1.0.3 are not recommended for other reasons.)

      When upgrading, please take a moment to read about [alternative Unicode back ends for `idna`](https://docs.rs/crate/idna_adapter/latest).

      If you are using Rust earlier than 1.81 in combination with SQLx 0.8.2 or earlier, please also read an [issue](servo/rust-url#992) about combining them with `url` 2.5.4 and `idna` 1.0.3.

      ## Additional information

      This issue resulted from `idna` 0.5.0 and earlier implementing the UTS 46 specification literally on this point and the specification having this bug. The specification bug has been fixed in [revision 33 of UTS 46](https://www.unicode.org/reports/tr46/tr46-33.html#Modifications).

      ## Acknowledgements

      Thanks to kageshiron for recognizing the security implications of this behavior.
    ├ Announcement: https://bugzilla.mozilla.org/show_bug.cgi?id=1887898
    ├ Solution: Upgrade to >=1.0.0 (try `cargo update -p idna`)
    ├ idna v0.1.5
      └── url v1.7.2
          └── hyper v0.10.16
              └── xml-rpc v0.0.12
                  └── rosrust v0.9.11
                      ├── arci-ros v0.1.0
                      │   ├── arci-ros-hygiene-test v0.0.0
                      │   └── openrr-apps v0.1.0
                      │       └── openrr v0.1.0
                      └── tf_rosrust v0.1.0
                          └── arci-ros v0.1.0 (*)
```
taiki-e added a commit to openrr/openrr that referenced this issue Dec 20, 2024
```
error[vulnerability]: `idna` accepts Punycode labels that do not produce any non-ASCII when decoded
    ┌─ /home/runner/work/openrr/openrr/Cargo.lock:260:1
    │
260 │ idna 0.1.5 registry+https://github.com/rust-lang/crates.io-index
    │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ security vulnerability detected
    │
    ├ ID: RUSTSEC-2024-0421
    ├ Advisory: https://rustsec.org/advisories/RUSTSEC-2024-0421
    ├ `idna` 0.5.0 and earlier accepts Punycode labels that do not produce any non-ASCII output, which means that either ASCII labels or the empty root label can be masked such that they appear unequal without IDNA processing or when processed with a different implementation and equal when processed with `idna` 0.5.0 or earlier.

      Concretely, `example.org` and `xn--example-.org` become equal after processing by `idna` 0.5.0 or earlier. Also, `example.org.xn--` and `example.org.` become equal after processing by `idna` 0.5.0 or earlier.

      In applications using `idna` (but not in `idna` itself) this may be able to lead to privilege escalation when host name comparison is part of a privilege check and the behavior is combined with a client that resolves domains with such labels instead of treating them as errors that preclude DNS resolution / URL fetching and with the attacker managing to introduce a DNS entry (and TLS certificate) for an `xn--`-masked name that turns into the name of the target when processed by `idna` 0.5.0 or earlier.

      ## Remedy

      Upgrade to `idna` 1.0.3 or later, if depending on `idna` directly, or to `url` 2.5.4 or later, if depending on `idna` via `url`. (This issue was fixed in `idna` 1.0.0, but versions earlier than 1.0.3 are not recommended for other reasons.)

      When upgrading, please take a moment to read about [alternative Unicode back ends for `idna`](https://docs.rs/crate/idna_adapter/latest).

      If you are using Rust earlier than 1.81 in combination with SQLx 0.8.2 or earlier, please also read an [issue](servo/rust-url#992) about combining them with `url` 2.5.4 and `idna` 1.0.3.

      ## Additional information

      This issue resulted from `idna` 0.5.0 and earlier implementing the UTS 46 specification literally on this point and the specification having this bug. The specification bug has been fixed in [revision 33 of UTS 46](https://www.unicode.org/reports/tr46/tr46-33.html#Modifications).

      ## Acknowledgements

      Thanks to kageshiron for recognizing the security implications of this behavior.
    ├ Announcement: https://bugzilla.mozilla.org/show_bug.cgi?id=1887898
    ├ Solution: Upgrade to >=1.0.0 (try `cargo update -p idna`)
    ├ idna v0.1.5
      └── url v1.7.2
          └── hyper v0.10.16
              └── xml-rpc v0.0.12
                  └── rosrust v0.9.11
                      ├── arci-ros v0.1.0
                      │   ├── arci-ros-hygiene-test v0.0.0
                      │   └── openrr-apps v0.1.0
                      │       └── openrr v0.1.0
                      └── tf_rosrust v0.1.0
                          └── arci-ros v0.1.0 (*)
```
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 a pull request may close this issue.

9 participants