Fix divide by zero when giving a tileset width of zero #292

smackysnacks · 2024-05-30T15:26:52Z

Input

<?xml version="1.0" encoding="UTF-8"?>
<map version="1.2" tiledversion="2020.05.20" orientation="orthogonal" width="100" height="100" tilewidth="32" tileheight="32">
 <tileset firstgid="1" name="tilesheet" tilewidth="0" tileheight="32" tilecount="84">
  <image source="tilesheet.png" width="448" height="192"/>
 </tileset>
</map>

thread '<unnamed>' panicked at /mnt/e/code/rs-tiled/src/tileset.rs:325:26:
attempt to divide by zero
stack backtrace:
   0:     0x55b58d6d5335 - std::backtrace_rs::backtrace::libunwind::trace::h9fff41df29930226
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/../../backtrace/src/backtrace/libunwind.rs:105:5
   1:     0x55b58d6d5335 - std::backtrace_rs::backtrace::trace_unsynchronized::hd333ee9fabe37696
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55b58d6d5335 - std::sys_common::backtrace::_print_fmt::ha8cea204a14d4b05
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/sys_common/backtrace.rs:68:5
   3:     0x55b58d6d5335 - <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:     0x55b58d7243bb - core::fmt::rt::Argument::fmt::h108a26a03f438748
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/core/src/fmt/rt.rs:165:63
   5:     0x55b58d7243bb - core::fmt::write::h04064cb45a345462
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/core/src/fmt/mod.rs:1172:21
   6:     0x55b58d6ca13f - std::io::Write::write_fmt::h222f9a473033d72e
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/io/mod.rs:1835:15
   7:     0x55b58d6d510e - std::sys_common::backtrace::_print::h0d72338fa5455ac9
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x55b58d6d510e - std::sys_common::backtrace::print::he02582a61c39a81d
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x55b58d6d7b39 - std::panicking::default_hook::{{closure}}::h93f8f6e01b4e4fd9
  10:     0x55b58d6d78da - std::panicking::default_hook::hbc1b8395cdf679f0
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:298:9
  11:     0x55b58d647e5a - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::hd3821373d4b335e9
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/alloc/src/boxed.rs:2077:9
  12:     0x55b58d647e5a - libfuzzer_sys::initialize::{{closure}}::h34544c005e9ab75c
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/src/lib.rs:90:9
  13:     0x55b58d6d826b - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h79ea62f4492f4aab
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/alloc/src/boxed.rs:2077:9
  14:     0x55b58d6d826b - std::panicking::rust_panic_with_hook::h2d3b3b5d41eafa75
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:799:13
  15:     0x55b58d6d7fab - std::panicking::begin_panic_handler::{{closure}}::hc78e06bc8800937b
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:656:13
  16:     0x55b58d6d57f9 - std::sys_common::backtrace::__rust_end_short_backtrace::h10081666adae5ac0
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/sys_common/backtrace.rs:171:18
  17:     0x55b58d6d7d17 - rust_begin_unwind
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:652:5
  18:     0x55b58d720983 - core::panicking::panic_fmt::he2bec51da94b7c97
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/core/src/panicking.rs:72:14
  19:     0x55b58d730057 - core::panicking::panic_const::panic_const_div_by_zero::hc376f41d21402289
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/core/src/panicking.rs:179:21
  20:     0x55b58d39e9c7 - tiled::tileset::Tileset::calculate_columns::{{closure}}::h3fd8321f69253a17
                               at /mnt/e/code/rs-tiled/src/tileset.rs:325:26
  21:     0x55b58d39e9c7 - core::option::Option<T>::map::h43fea153502e6eae
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/core/src/option.rs:1072:29
  22:     0x55b58d39e9c7 - tiled::tileset::Tileset::calculate_columns::h8238c47272922130
                               at /mnt/e/code/rs-tiled/src/tileset.rs:325:14
  23:     0x55b58d2b3e58 - tiled::tileset::Tileset::finish_parsing_xml::{{closure}}::h19fb8ad287b2a987
                               at /mnt/e/code/rs-tiled/src/tileset.rs:297:32
  24:     0x55b58d2b3e58 - core::option::Option<T>::unwrap_or_else::h28bba47334ff9cc8
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/core/src/option.rs:977:21
  25:     0x55b58d2b3e58 - tiled::tileset::Tileset::finish_parsing_xml::hfce72c5b003bf24e
                               at /mnt/e/code/rs-tiled/src/tileset.rs:297:14
  26:     0x55b58d2b8d7f - tiled::tileset::Tileset::parse_xml_embedded::h2142a7c18dd9559a
                               at /mnt/e/code/rs-tiled/src/tileset.rs:158:9
  27:     0x55b58d278893 - tiled::tileset::Tileset::parse_xml_in_map::h60a55f37a6335810
                               at /mnt/e/code/rs-tiled/src/tileset.rs:120:9
  28:     0x55b58d278893 - tiled::map::Map::parse_xml::{{closure}}::hb84ad166f56bf5f4
                               at /mnt/e/code/rs-tiled/src/map.rs:170:27
  29:     0x55b58d273745 - tiled::map::Map::parse_xml::hda629daa56e91da8
                               at /mnt/e/code/rs-tiled/src/util.rs:189:60
  30:     0x55b58d284bfb - tiled::parse::xml::map::parse_map::hefdeda8e31c451cc
                               at /mnt/e/code/rs-tiled/src/parse/xml/map.rs:27:28
  31:     0x55b58d314729 - tiled::loader::Loader<Cache,Reader>::load_tmx_map::h010047c7233d8644
                               at /mnt/e/code/rs-tiled/src/loader.rs:169:9
  32:     0x55b58d314729 - tiled::_::__libfuzzer_sys_run::h2c36ea867085381d
                               at /mnt/e/code/rs-tiled/fuzz/fuzz_targets/tiled.rs:31:13
  33:     0x55b58d313e87 - rust_fuzzer_test_input
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/src/lib.rs:224:17
  34:     0x55b58d642e30 - libfuzzer_sys::test_input_wrap::{{closure}}::h2166b5ce337a04af
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/src/lib.rs:61:9
  35:     0x55b58d642e30 - std::panicking::try::do_call::hf9a8dd70a57cf361
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:559:40
  36:     0x55b58d648078 - __rust_try
  37:     0x55b58d64759c - std::panicking::try::h14ad3ab26713fe2c
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panicking.rs:523:19
  38:     0x55b58d64759c - std::panic::catch_unwind::hd0baf9db6bc5566c
                               at /rustc/36153f1a4e3162f0a143c7b3e468ecb3beb0008e/library/std/src/panic.rs:149:14
  39:     0x55b58d64759c - LLVMFuzzerTestOneInput
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/src/lib.rs:59:22
  40:     0x55b58d66d596 - _ZN6fuzzer6Fuzzer15ExecuteCallbackEPKhm
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/libfuzzer/FuzzerLoop.cpp:612:15
  41:     0x55b58d64d827 - _ZN6fuzzer10RunOneTestEPNS_6FuzzerEPKcm
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/libfuzzer/FuzzerDriver.cpp:324:21
  42:     0x55b58d656763 - _ZN6fuzzer12FuzzerDriverEPiPPPcPFiPKhmE
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/libfuzzer/FuzzerDriver.cpp:860:19
  43:     0x55b58d6480d7 - main
                               at /home/smacks/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libfuzzer-sys-0.4.7/libfuzzer/FuzzerMain.cpp:20:30
  44:     0x7fb392bf4d90 - <unknown>
  45:     0x7fb392bf4e40 - __libc_start_main
  46:     0x55b58d130a65 - _start
  47:                0x0 - <unknown>

@bjorn - The fix in this PR assumes that a Tileset may never have a tile width or tile height of zero. Is this a safe assumption? The actual panic caused by this input occurs here, but we should be able to error out earlier in the tileset parsing if my assumption is correct.

bjorn

Good catch! However, the division by zero (and the related check against it) should only happen when the tileset is referencing an image containing the tiles.

Tiled also supports "collection of images" tilesets, where each tile refers to its own image. In this case, the tilewidth and tileheight properties store the size of the largest dimensions (it's rather pointless, I should probably not have included these attributes at all in this case) and their values may be 0 when the tileset is an image collection tileset without any tiles added to it.

So I'm afraid this check will have to be delayed until we can also check whether Tileset::image has a value. Probably we can add it to this block:

rs-tiled/src/tileset.rs

Lines 286 to 290 in 15a0b4a

    
           if !is_image_collection_tileset { 
        
               for tile_id in 0..prop.tilecount { 
        
                   tiles.entry(tile_id).or_default(); 
        
               } 
        
           }

Fixes a divide by zero panic

smackysnacks · 2024-06-06T16:38:04Z

Moving the check within that block worked to fix the divide by zero

bjorn requested changes May 31, 2024

View reviewed changes

bjorn mentioned this pull request May 31, 2024

Fix panic in ObjectData on unexpected XML in content #291

Merged

fix: disallow tileset width/height from being 0

af38a83

Fixes a divide by zero panic

smackysnacks force-pushed the fix/dividebyzero-tileset-widthheight branch from 984067d to af38a83 Compare June 6, 2024 16:36

smackysnacks requested a review from bjorn June 6, 2024 23:01

bjorn approved these changes Jun 7, 2024

View reviewed changes

aleokdev merged commit 6677c4d into mapeditor:next Jun 7, 2024
3 checks passed

smackysnacks deleted the fix/dividebyzero-tileset-widthheight branch June 7, 2024 14:15

bjorn mentioned this pull request Jun 21, 2024

docs: Be more specific about the tilewidth/height attributes mapeditor/tiled#3981

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix divide by zero when giving a tileset width of zero #292

Fix divide by zero when giving a tileset width of zero #292

smackysnacks commented May 30, 2024 •

edited

Loading

bjorn left a comment •

edited

Loading

smackysnacks commented Jun 6, 2024 •

edited

Loading

	if !is_image_collection_tileset {
	for tile_id in 0..prop.tilecount {
	tiles.entry(tile_id).or_default();
	}
	}

Fix divide by zero when giving a tileset width of zero #292

Fix divide by zero when giving a tileset width of zero #292

Conversation

smackysnacks commented May 30, 2024 • edited Loading

bjorn left a comment • edited Loading

Choose a reason for hiding this comment

smackysnacks commented Jun 6, 2024 • edited Loading

smackysnacks commented May 30, 2024 •

edited

Loading

bjorn left a comment •

edited

Loading

smackysnacks commented Jun 6, 2024 •

edited

Loading