URL:
<http://gna.org/bugs/?21489>
Summary: Scenarios with missing [time] schedule result in
invalid memory access
Project: Battle for Wesnoth
Submitted by: shadowmaster
Submitted on: Thu 16 Jan 2014 01:52:04 AM CLST
Category: Bug
Severity: 5 - Blocker
Priority: 5 - Normal
Item Group: WML
Status: None
Privacy: Public
Assigned to: fendrin
Originator Email:
Open/Closed: Open
Discussion Lock: Any
Release: 1.11.8, 1.11.8+dev
Operating System: Any
_______________________________________________________
Details:
In Wesnoth 1.11.8, a scenario without any [time] tags defining a valid ToD
schedule will trigger undefined behavior from Wesnoth under certain
conditions. For example, if the [store_time_of_day] WML action is used, a NULL
pointer dereference may take place. This is definitely not limited to that WML
action and not limited to a NULL pointer dereference, as evidenced by the
following steps to reproduce:
* Edit HttT scenario 1 to remove the {DEFAULT_SCHEDULE} macro inclusion.
* Start HttT, skip the storyscreen.
* The game should crash.
Example backtrace for this test case:
(gdb) bt
#0 0x0000000000e27823 in tod_color::operator+ (this=0x53fcf00, o=...) at
src/time_of_day.hpp:35
#1 0x0000000000e16d5b in display::update_tod (this=0x53fc720) at
src/display.cpp:518
#2 0x0000000000b9eb48 in play_controller::init_gui (this=0x7fffffffbb50) at
src/play_controller.cpp:585
#3 0x0000000000655ab3 in playsingle_controller::init_gui
(this=0x7fffffffbb50) at src/playsingle_controller.cpp:93
#4 0x0000000000657955 in playsingle_controller::play_scenario
(this=0x7fffffffbb50, story=..., skip_replay=false) at
src/playsingle_controller.cpp:376
#5 0x000000000064a24b in playsingle_scenario (game_config=...,
level=0x4eb8820, disp=..., state_of_game=..., story=..., skip_replay=false,
end_level=...) at src/playcampaign.cpp:243
#6 0x000000000064c1c9 in play_game (disp=..., gamestate=..., game_config=...,
io_type=IO_NONE, skip_replay=false, network_game=false) at
src/playcampaign.cpp:462
#7 0x00000000004974ca in game_controller::launch_game (this=0x199a5d0,
reload=game_controller::RELOAD_DATA) at src/game_controller.cpp:955
#8 0x000000000041e11d in do_gameloop (argc=1, argv=0x7fffffffe0b8) at
src/game.cpp:643
#9 0x000000000041e616 in main (argc=1, argv=0x7fffffffe0b8) at
src/game.cpp:699
In frame 0:
(gdb) frame 0
#0 0x0000000000e27823 in tod_color::operator+ (this=0x53fcf00, o=...) at
src/time_of_day.hpp:35
35 tod_color operator+(const tod_color& o) const { return
tod_color(r+o.r, g+o.g, b+o.b);}
(gdb) p &o
$1 = (const tod_color *) 0x28
It is evident that the reference passed to the method is invalid and offset
from NULL. This is because the reference is to a member of a parent object
that *is* a NULL reference:
(gdb) frame 1
#1 0x0000000000e16d5b in display::update_tod (this=0x53fc720) at
src/display.cpp:518
518 tod_color col = color_adjust_ + tod.color;
(gdb) p &tod
$3 = (const time_of_day *) 0x0
A git bisect indicates that this is caused by commit
9a44642689aacb747ba9acbc7aa3826f6c510845
<https://github.com/wesnoth/wesnoth-old/commit/9a44642689aacb747ba9acbc7aa3826f6c510845>,
possibly (this is just speculation on my part) because of the removal of the
default-construction of a time_of_day object in time_of_day::parse_times() in
an if conditional block. I suspect most code expects scenarios to have at
least one ToD defined, so without the default-constructed ToD fallback, the
engine may be dereferencing end iterators.
While one could argue that Wesnoth shouldn't support scenarios without a
defined ToD schedule, this actually works fine on 1.10.x through 1.11.7 up to
the aforementioned commit, and WML should not be under *any* circumstances
allowed to crash the game or otherwise make it incur in undefined behavior.
Additionally, there is at least one instance in mainline of a schedule-less
scenario in the form of HttT's epilogue scenario, although it doesn't crash --
presumably because, unlike the example above, it ends the scenario during the
prestart event, before the game UI (which needs to know the current ToD) can
be initialized and displayed.
Existing scenarios presenting this issue can be fixed by defining a time
schedule consisting of a default-constructed ToD:
[time][/time]
However, this change of behavior isn't documented anywhere and it is clearly
both unintended and catastrophic to the normal operation of the game,
therefore it should be fixed.
_______________________________________________________
Reply to this item at:
<http://gna.org/bugs/?21489>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Wesnoth-bugs mailing list
[email protected]
https://mail.gna.org/listinfo/wesnoth-bugs
