tafia / quick-xml

Rust high performance xml reader and writer
MIT License
1.22k stars 237 forks source link

Panic when trim text is active #94

Closed CryZe closed 7 years ago

CryZe commented 7 years ago

When parsing the following XML

<Run>
<!B>
</Run>

with reader.trim_text(true);, then it panics with:

        thread 'parse::livesplit_fuzz_crash' panicked at 'index 5 out of range for slice of length 4', src\libcore\slice\mod.rs:748:4
stack backtrace:
   0:           0xd563cf - std::sys_common::backtrace::_print::h788ffa2f5ef861b7
                               at src\libstd\sys\windows\backtrace/mod.rs:65
                               at src\libstd\sys_common/backtrace.rs:71
   1:           0xd67b76 - std::panicking::default_hook::{{closure}}::h07b19ee06ce409af
                               at src\libstd\sys_common/backtrace.rs:60
                               at src\libstd/panicking.rs:381
   2:           0xd6785a - std::panicking::default_hook::h5f4807df908f2485
                               at src\libstd/panicking.rs:391
   3:           0xd6803c - std::panicking::rust_panic_with_hook::h4b94a3a8ebb03363
                               at src\libstd/panicking.rs:611
   4:           0xd67f08 - std::panicking::begin_panic::hc1afa6ec7886430a
                               at src\libstd/panicking.rs:572
   5:           0xd67deb - std::panicking::begin_panic_fmt::h745ccd445d0a2438
                               at src\libstd/panicking.rs:522
   6:           0xd67d6f - rust_begin_unwind
                               at src\libstd/panicking.rs:498
   7:           0xd7b01e - core::panicking::panic_fmt::h5c59f72fc2fa4c71
                               at src\libcore/panicking.rs:71
   8:           0xd7b111 - core::slice::slice_index_len_fail::h0eddb005211069d8
                               at src\libcore\slice/mod.rs:748
   9:           0x4a1821 - <core::ops::range::Range<usize> as core::slice::SliceIndex<[T]>>::index::hd1fc40186de97550
                               at C:\projects\rust\src\libcore\slice/mod.rs:879
  10:           0x47c5b0 - core::slice::<impl core::ops::index::Index<I> for [T]>::index::hdb6bef459e9f75c3
                               at C:\projects\rust\src\libcore\slice/mod.rs:730
  11:           0x40265c - <alloc::vec::Vec<T> as core::ops::index::Index<core::ops::range::Range<usize>>>::index::hec15d839b3cc4514
                               at C:\projects\rust\src\liballoc/vec.rs:1575
  12:           0x45da41 - <quick_xml::reader::Reader<B>>::read_bang::hbb0897709c9e1f08
                               at C:\Users\Christopher Serr\.cargo\registry\src\github.com-1ecc6299db9ec823\quick-xml-0.9.3\src/reader.rs:277
  13:           0x45c723 - <quick_xml::reader::Reader<B>>::read_until_close::h364d8540707f02bd
                               at C:\Users\Christopher Serr\.cargo\registry\src\github.com-1ecc6299db9ec823\quick-xml-0.9.3\src/reader.rs:224
  14:           0x45b00a - <quick_xml::reader::Reader<B>>::read_event::h2d8f1f169a3604f1
                               at C:\Users\Christopher Serr\.cargo\registry\src\github.com-1ecc6299db9ec823\quick-xml-0.9.3\src/reader.rs:454
  15:           0x45baa2 - <quick_xml::reader::Reader<B>>::read_until_open::hab15a807d188a780
                               at C:\Users\Christopher Serr\.cargo\registry\src\github.com-1ecc6299db9ec823\quick-xml-0.9.3\src/reader.rs:167
  16:           0x45b01d - <quick_xml::reader::Reader<B>>::read_event::h2d8f1f169a3604f1
                               at C:\Users\Christopher Serr\.cargo\registry\src\github.com-1ecc6299db9ec823\quick-xml-0.9.3\src/reader.rs:455
  17:           0x40ad84 - livesplit_core::run::parser::quick_xml_util::parse_children::h655e1cbb19ec474e
                               at C:\Projekte\livesplit-core\src\run\parser/quick_xml_util.rs:193
  18:           0x446339 - livesplit_core::run::parser::quick_livesplit::parse::{{closure}}::hf7a50f3cece4b789
                               at C:\Projekte\livesplit-core\src\run\parser/quick_livesplit.rs:354
  19:           0x404511 - livesplit_core::run::parser::quick_xml_util::parse_base::h8be9f426055b57a7
                               at C:\Projekte\livesplit-core\src\run\parser/quick_xml_util.rs:219
  20:           0x445c6d - livesplit_core::run::parser::quick_livesplit::parse::had2e9529462e5375
                               at C:\Projekte\livesplit-core\src\run\parser/quick_livesplit.rs:346
  21:           0x4a2e7b - parsing::parse::livesplit_fuzz_crash::h739424198cfa3d47
                               at tests/parsing.rs:22
  22:           0x4b38c6 - <F as test::FnBox<T>>::call_box::h8652f97c00eee037
                               at src\libtest/lib.rs:1480
                               at C:\projects\rust\src\libcore\ops/function.rs:223
                               at src\libtest/lib.rs:141
  23:           0xd6b6ce - _rust_maybe_catch_panic
                               at src\libpanic_unwind/lib.rs:99
  24:           0x4a406c - std::sys_common::backtrace::__rust_begin_short_backtrace::h6087b56ec1e9aa74
                               at C:\projects\rust\src\libstd/panicking.rs:459
                               at C:\projects\rust\src\libstd/panic.rs:361
                               at src\libtest/lib.rs:1419
                               at C:\projects\rust\src\libstd\sys_common/backtrace.rs:136
  25:           0x4a4ce7 - std::panicking::try::do_call::hd464f7013f49753a
                               at C:\projects\rust\src\libstd\thread/mod.rs:394
                               at C:\projects\rust\src\libstd/panic.rs:296
                               at C:\projects\rust\src\libstd/panicking.rs:480
  26:           0xd6b6ce - _rust_maybe_catch_panic
                               at src\libpanic_unwind/lib.rs:99
  27:           0x4ac9ac - <F as alloc::boxed::FnBox<A>>::call_box::h8657cf680c887452
                               at C:\projects\rust\src\libstd/panicking.rs:459
                               at C:\projects\rust\src\libstd/panic.rs:361
                               at C:\projects\rust\src\libstd\thread/mod.rs:393
                               at C:\projects\rust\src\liballoc/boxed.rs:682
  28:           0xd65f90 - std::sys::imp::thread::Thread::new::thread_start::h58762d9deebb6e43
                               at C:\projects\rust\src\liballoc/boxed.rs:692
                               at src\libstd\sys_common/thread.rs:21
                               at src\libstd\sys\windows/thread.rs:51
  29:     0x7fff0ff12773 - unit_addrs_search
tafia commented 7 years ago

Thanks for the issue! Iĺl look at this asap.

CryZe commented 7 years ago

I'm kinda trying to figure it out atm too, but you may be faster. Here's some code to reproduce it:

extern crate quick_xml;

use quick_xml::reader::Reader;
use quick_xml::events::Event;

fn main() {
    let src = br#"<Run>
<!B>
</Run>"#;
    let mut reader = Reader::from_reader(&src[..]);
    reader.trim_text(true);

    let mut buf = Vec::new();

    loop {
        if let Event::Eof = reader.read_event(&mut buf).unwrap() {
            break;
        }
        buf.clear();
    }
}
CryZe commented 7 years ago
if len >= 3 && &buf[buf_start + 1..buf_start + 3] == b"--" {

this line and other lines in the read_bang method implicitly assume that buf_start is 0. So the len check here should be len >= buf_start + 3 for example (at least that's what it should be to not panic). So it probably makes sense for you to put in the proper logic, now that you know what the bug is.

Actually, maybe it makes sense to preslice the whole buf, so this can't even happen, instead of adding the buf_start to every indexing / range operation.

tafia commented 7 years ago

yes, all these tests should be

if len >= buf_start + x { ... }
tafia commented 7 years ago

I have done it as well. Do you want to do a PR or I'll do it?

CryZe commented 7 years ago

nah, you should probably do it. I'm not familiar with all the specifics of the method enough to be comfortable with it being correct.

tafia commented 7 years ago

sure, thanks!

CryZe commented 7 years ago

Nice, thank you :)

tafia commented 7 years ago

Published v0.9.4 Thanks again for the issue!!