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

Panic when trim text is active #94

Closed
CryZe opened this issue Sep 22, 2017 · 9 comments
Closed

Panic when trim text is active #94

CryZe opened this issue Sep 22, 2017 · 9 comments

Comments

@CryZe
Copy link
Contributor

CryZe commented Sep 22, 2017

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
Copy link
Owner

tafia commented Sep 23, 2017

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

@CryZe
Copy link
Contributor Author

CryZe commented Sep 23, 2017

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
Copy link
Contributor Author

CryZe commented Sep 23, 2017

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
Copy link
Owner

tafia commented Sep 23, 2017

yes, all these tests should be

if len >= buf_start + x { ... }

@tafia
Copy link
Owner

tafia commented Sep 23, 2017

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

@CryZe
Copy link
Contributor Author

CryZe commented Sep 23, 2017

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
Copy link
Owner

tafia commented Sep 23, 2017

sure, thanks!

@CryZe
Copy link
Contributor Author

CryZe commented Sep 23, 2017

Nice, thank you :)

@tafia
Copy link
Owner

tafia commented Sep 23, 2017

Published v0.9.4
Thanks again for the issue!!

Mingun added a commit to Mingun/quick-xml that referenced this issue Sep 29, 2023
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

No branches or pull requests

2 participants