search

mapeditor / rs-tiled

Reads files from the Tiled editor into Rust
https://crates.io/crates/tiled
MIT License
268 stars 102 forks source link

Fix panic in ObjectData on unexpected XML in content #291

Closed smackysnacks closed 6 months ago

smackysnacks commented 6 months ago

Input:

<?xml version="1.0" encoding="UTF-8"?>
<map version="1.10" tiledversion="1.10.2" orientation="orthogonal" renderorder="right-down" width="1" height="1" tilewidth="16" tileheight="16" infinite="0" nextlayerid="3" nextobjectid="2">
 <objectgroup d="2">
 <objectgroup id="2" name="Object Layer 1">
  <object idy="-2.39844" width="87.7188" height="21.7969">
   <text coloi="#6455ff7f" bold="1" italic="1" u="right-down" width="1" heigid="2" name="Object Layer 1">
  <o455ff7f" bold="1" italic="1" u="right-down" width="1" heighlig/object>
 </objectgroup>
</map>
fuzz/target/x86_64-unknown-linux-gnu/debug/tiled: Running 1 inputs 1 time(s) each.
Running: fuzz/artifacts/tiled/minimized-from-c0b0d99494579d1338897e64a21e0ce666e75fbd
thread '<unnamed>' panicked at /mnt/e/code/rs-tiled/src/objects.rs:457:18:
Text attribute contained anything but characters as content
stack backtrace:
   0:     0x55df840aec15 - std::backtrace_rs::backtrace::libunwind::trace::h9fff41df29930226
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/../../backtrace/src/backtrace/libunwind.rs:105:5
   1:     0x55df840aec15 - std::backtrace_rs::backtrace::trace_unsynchronized::hd333ee9fabe37696
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55df840aec15 - std::sys_common::backtrace::_print_fmt::ha8cea204a14d4b05
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/sys_common/backtrace.rs:68:5
   3:     0x55df840aec15 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h23b6b3b597037ccc
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x55df840fdc9b - core::fmt::rt::Argument::fmt::h108a26a03f438748
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/core/src/fmt/rt.rs:165:63
   5:     0x55df840fdc9b - core::fmt::write::h04064cb45a345462
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/core/src/fmt/mod.rs:1172:21
   6:     0x55df840a3a1f - std::io::Write::write_fmt::h222f9a473033d72e
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/io/mod.rs:1835:15
   7:     0x55df840ae9ee - std::sys_common::backtrace::_print::h0d72338fa5455ac9
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x55df840ae9ee - std::sys_common::backtrace::print::he02582a61c39a81d
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x55df840b1419 - std::panicking::default_hook::{{closure}}::h93f8f6e01b4e4fd9
  10:     0x55df840b11ba - std::panicking::default_hook::hbc1b8395cdf679f0
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:298:9
  11:     0x55df83fc0690 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h0913f36c163d89aa
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/alloc/src/boxed.rs:2077:9
  12:     0x55df83fc3087 - libfuzzer_sys::initialize::{{closure}}::hc58cb4b6be71cc7d
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/src/lib.rs:90:9
  13:     0x55df840b1b4b - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h79ea62f4492f4aab
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/alloc/src/boxed.rs:2077:9
  14:     0x55df840b1b4b - std::panicking::rust_panic_with_hook::h2d3b3b5d41eafa75
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:799:13
  15:     0x55df83a4ea9f - std::panicking::begin_panic::{{closure}}::h9bb24efce3846101
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:694:9
  16:     0x55df83a22048 - std::sys_common::backtrace::__rust_end_short_backtrace::hbde81386f36a5c08
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/sys_common/backtrace.rs:171:18
  17:     0x55df83a4e7e7 - std::panicking::begin_panic::h2190585fac38e662
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:693:12
  18:     0x55df839e6cc4 - tiled::objects::ObjectData::new_text::h55d705fcac1894db
                               at /mnt/e/code/rs-tiled/src/objects.rs:457:18
  19:     0x55df839e4f11 - tiled::objects::ObjectData::new::{{closure}}::hedaa150121b31586
                               at /mnt/e/code/rs-tiled/src/objects.rs:313:30
  20:     0x55df839da2dc - tiled::objects::ObjectData::new::h07d05473377ac2e5
                               at /mnt/e/code/rs-tiled/src/util.rs:189:60
  21:     0x55df83a29c05 - tiled::layers::object::ObjectLayerData::new::{{closure}}::hb773f54d3d3ba89f
                               at /mnt/e/code/rs-tiled/src/layers/object.rs:43:30
  22:     0x55df83a27321 - tiled::layers::object::ObjectLayerData::new::h4c8824280353dea4
                               at /mnt/e/code/rs-tiled/src/util.rs:189:60
  23:     0x55df83aa6282 - tiled::layers::LayerData::new::hfcb3245cd1958a2c
                               at /mnt/e/code/rs-tiled/src/layers/mod.rs:116:40
  24:     0x55df83a9857a - tiled::map::Map::parse_xml::{{closure}}::hac2fe4e34ec07ec9
                               at /mnt/e/code/rs-tiled/src/map.rs:218:29
  25:     0x55df83a8ee16 - tiled::map::Map::parse_xml::h1d113b9054255d38
                               at /mnt/e/code/rs-tiled/src/util.rs:189:60
  26:     0x55df83a403ed - tiled::parse::xml::map::parse_map::h28ea4b43dc040a33
                               at /mnt/e/code/rs-tiled/src/parse/xml/map.rs:27:28
  27:     0x55df839c91f5 - tiled::loader::Loader<Cache,Reader>::load_tmx_map::h123d31838d954b75
                               at /mnt/e/code/rs-tiled/src/loader.rs:169:9
  28:     0x55df83ae2011 - tiled::_::__libfuzzer_sys_run::he5c7aa3d48cb477e
                               at /mnt/e/code/rs-tiled/fuzz/fuzz_targets/tiled.rs:31:13
  29:     0x55df83ae1a56 - rust_fuzzer_test_input
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/src/lib.rs:224:17
  30:     0x55df83fc2865 - libfuzzer_sys::test_input_wrap::{{closure}}::h92923ccbb00bba0c
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/src/lib.rs:61:9
  31:     0x55df83fbc8cd - std::panicking::try::do_call::h34f81e05f62ad1f6
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:559:40
  32:     0x55df83fc33db - __rust_try
  33:     0x55df83fbc56f - std::panicking::try::h643cc48aebb6fcb6
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:523:19
  34:     0x55df83fbc3b6 - std::panic::catch_unwind::h3e1fa6f6ac0fe63a
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panic.rs:149:14
  35:     0x55df83fc2544 - LLVMFuzzerTestOneInput
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/src/lib.rs:59:22
  36:     0x55df83ff6fd4 - _ZN6fuzzer6Fuzzer15ExecuteCallbackEPKhm
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/libfuzzer/FuzzerLoop.cpp:612:15
  37:     0x55df83fc61a4 - _ZN6fuzzer10RunOneTestEPNS_6FuzzerEPKcm
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/libfuzzer/FuzzerDriver.cpp:324:21
  38:     0x55df83fcac2e - _ZN6fuzzer12FuzzerDriverEPiPPPcPFiPKhmE
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/libfuzzer/FuzzerDriver.cpp:860:19
  39:     0x55df83fc3454 - main
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/libfuzzer/FuzzerMain.cpp:20:30
  40:     0x7f1a180cfd90 - <unknown>
  41:     0x7f1a180cfe40 - __libc_start_main
  42:     0x55df838f4425 - _start
  43:                0x0 - <unknown>
smackysnacks commented 6 months ago

@aleokdev btw, I'm looking at an unrelated crash (divide by zero) and was wondering if a tile is ever allowed to have a height or width of 0? If not, then we should be able to error out early when coming across such a tile.

aleokdev commented 6 months ago

@aleokdev btw, I'm looking at an unrelated crash (divide by zero) and was wondering if a tile is ever allowed to have a height or width of 0? If not, then we should be able to error out early when coming across such a tile.

I'm not sure, the TMX reference doesn't seem to say anything; @bjorn maybe knows

bjorn commented 6 months ago

I'm not sure, the TMX reference doesn't seem to say anything; @bjorn maybe knows

Answered at https://github.com/mapeditor/rs-tiled/pull/292#pullrequestreview-2090246387. I'll add a note about this in the reference (done in https://github.com/mapeditor/tiled/pull/3981).