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

IntoIterator for Tokens is unsound #105

Closed
Manishearth opened this issue Oct 30, 2023 · 0 comments · Fixed by #106 or rust-lang/rust#117488
Closed

IntoIterator for Tokens is unsound #105

Manishearth opened this issue Oct 30, 2023 · 0 comments · Fixed by #106 or rust-lang/rust#117488

Comments

@Manishearth
Copy link

minifier-rs/src/js/token.rs

Lines 1076 to 1089 in c8e8bee

impl<'a> Iterator for IntoIterTokens<'a> {
type Item = (Token<'a>, Option<&'a Token<'a>>);
fn next(&mut self) -> Option<Self::Item> {
if self.inner.0.is_empty() {
None
} else {
let ret = self.inner.0.pop().expect("pop() failed");
// FIXME once generic traits' types are stabilized, use a second
// lifetime instead of transmute!
Some((ret, unsafe { std::mem::transmute(self.inner.0.last()) }))
}
}
}

This produces a lifetime much longer than correct: it produces a reference to vec data that has a local lifetime but extends that to 'a.

Furthermore, since this is in an IntoIterator impl, most of the time this gets used it will be unsound: the vector is dropped immediately after IntoIterator, but these references could potentially be held on to for much longer.

If the Iterator API cannot express what is necessary here, we should not use the iterator API.

Is there a reason that second tuple field is needed anyway? It feels like we can do better exposing a .last() method or something.

use minifier::js;

fn main() {
    let source = "function foo() {}";
    let tokens = js::tokenize(source);
    let token = tokens.into_iter().next().unwrap();


    println!("{:?}", token);
}

fails on miri:

$ cargo +nightly miri run --example example
Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `/home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/debug/examples/example`
error: Undefined Behavior: constructing invalid value: encountered a dangling reference (use-after-free)
    --> /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2278:1
     |
2278 | fmt_refs! { Debug, Display, Octal, Binary, LowerHex, UpperHex, LowerExp, UpperExp }
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a dangling reference (use-after-free)
     |
     = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
     = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
     = note: BACKTRACE:
     = note: inside `<&minifier::js::Token<'_> as std::fmt::Debug>::fmt` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2268:71: 2268:78
     = note: inside `<&&minifier::js::Token<'_> as std::fmt::Debug>::fmt` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2268:62: 2268:82
     = note: inside closure at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/builders.rs:322:17: 322:36
     = note: inside `std::result::Result::<(), std::fmt::Error>::and_then::<(), [closure@std::fmt::DebugTuple<'_, '_>::field::{closure#0}]>` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/result.rs:1319:22: 1319:27
     = note: inside `std::fmt::DebugTuple::<'_, '_>::field` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/builders.rs:309:23: 324:11
     = note: inside `std::fmt::Formatter::<'_>::debug_tuple_field1_finish` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2035:9: 2035:30
     = note: inside `<std::option::Option<&minifier::js::Token<'_>> as std::fmt::Debug>::fmt` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:559:37: 559:42
     = note: inside `<&std::option::Option<&minifier::js::Token<'_>> as std::fmt::Debug>::fmt` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2268:62: 2268:82
     = note: inside closure at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/builders.rs:322:17: 322:36
     = note: inside `std::result::Result::<(), std::fmt::Error>::and_then::<(), [closure@std::fmt::DebugTuple<'_, '_>::field::{closure#0}]>` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/result.rs:1319:22: 1319:27
     = note: inside `std::fmt::DebugTuple::<'_, '_>::field` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/builders.rs:309:23: 324:11
     = note: inside `<(minifier::js::Token<'_>, std::option::Option<&minifier::js::Token<'_>>) as std::fmt::Debug>::fmt` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2461:25: 2461:46
     = note: inside `core::fmt::rt::Argument::<'_>::fmt` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/rt.rs:138:9: 138:40
     = note: inside `std::fmt::write` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:1094:17: 1094:40
     = note: inside `<std::io::StdoutLock<'_> as std::io::Write>::write_fmt` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/mod.rs:1714:15: 1714:43
     = note: inside `<&std::io::Stdout as std::io::Write>::write_fmt` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:726:9: 726:36
     = note: inside `<std::io::Stdout as std::io::Write>::write_fmt` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:700:9: 700:33
     = note: inside `std::io::stdio::print_to::<std::io::Stdout>` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:1018:21: 1018:47
     = note: inside `std::io::_print` at /home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:1095:5: 1095:37
note: inside `main`
    --> examples/example.rs:11:5
     |
11   |     println!("{:?}", token);
     |     ^^^^^^^^^^^^^^^^^^^^^^^
     = note: this error originates in the macro `fmt_refs` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 1, 2023
…notriddle

Update minifier-rs version to 0.3.0

It fixes GuillaumeGomez/minifier-rs#105.

r? `@notriddle`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 1, 2023
…notriddle

Update minifier-rs version to 0.3.0

It fixes GuillaumeGomez/minifier-rs#105.

r? ``@notriddle``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 1, 2023
…notriddle

Update minifier-rs version to 0.3.0

It fixes GuillaumeGomez/minifier-rs#105.

r? ```@notriddle```
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 2, 2023
Rollup merge of rust-lang#117488 - GuillaumeGomez:update-minifier, r=notriddle

Update minifier-rs version to 0.3.0

It fixes GuillaumeGomez/minifier-rs#105.

r? ```@notriddle```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant