GuillaumeGomez / minifier-rs

Minifier tool/lib for JS/CSS/JSON files
MIT License
86 stars 16 forks source link

IntoIterator for Tokens is unsound #105

Closed Manishearth closed 10 months ago

Manishearth commented 10 months ago

https://github.com/GuillaumeGomez/minifier-rs/blob/c8e8bee18b0ecca0603afdfd5f9c259f700deb72/src/js/token.rs#L1076-L1089

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 `> 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 ` 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 `::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::` 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 ```