Skip to content

Commit

Permalink
ChangeLevel: Fix out of bounds access to NPC extra_rotate data in saves
Browse files Browse the repository at this point in the history
This affects both reading and writing saves so could corrupt both the
rotation angles as well as the first stacked target. The write also
touches the saved blood color but that is overwritten with the correct
value later.

Only affects NPCs with extra rotation data - i.e. those that hat the
look around (-l) or stare at (-a) behaviors active at some point.

Was broken in commit 239cff7.

Fixes: issue #1668
  • Loading branch information
dscharrer committed Jan 5, 2023
1 parent 66d5852 commit 6a970e0
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
6 changes: 5 additions & 1 deletion src/scene/ChangeLevel.cpp
Expand Up @@ -2068,7 +2068,7 @@ static Entity * ARX_CHANGELEVEL_Pop_IO(std::string_view idString, EntityInstance
io->_npcdata->ex_rotate = new EERIE_EXTRA_ROTATE();
}
static_assert(SAVED_MAX_EXTRA_ROTATE <= MAX_EXTRA_ROTATE, "array size mismatch");
for(size_t i = 0; i < MAX_EXTRA_ROTATE; i++) {
for(size_t i = 0; i < SAVED_MAX_EXTRA_ROTATE; i++) {
if(!io->obj ||
as->ex_rotate.group_number[i] < 0 ||
size_t(as->ex_rotate.group_number[i]) >= io->obj->grouplist.size()) {
Expand All @@ -2080,6 +2080,10 @@ static Entity * ARX_CHANGELEVEL_Pop_IO(std::string_view idString, EntityInstance
io->_npcdata->ex_rotate->group_rotate[i] = as->ex_rotate.group_rotate[i];
}
}
for(size_t i = SAVED_MAX_EXTRA_ROTATE; i < MAX_EXTRA_ROTATE; i++) {
io->_npcdata->ex_rotate->group_number[i] = { };
io->_npcdata->ex_rotate->group_rotate[i] = { };
}
}

io->_npcdata->blood_color = Color::fromBGRA(ColorBGRA(as->blood_color));
Expand Down
5 changes: 3 additions & 2 deletions src/scene/SaveFormat.h
Expand Up @@ -768,11 +768,12 @@ struct SavedExtraRotate {
SavedExtraRotate & operator=(const EERIE_EXTRA_ROTATE & b) {
flags = 0;
static_assert(SAVED_MAX_EXTRA_ROTATE <= MAX_EXTRA_ROTATE, "array size mismatch");
std::transform(b.group_number.begin(), b.group_number.end(), group_number, [](VertexGroupId group) {
std::transform(b.group_number.begin(), b.group_number.begin() + SAVED_MAX_EXTRA_ROTATE, group_number,
[](VertexGroupId group) {
// TODO this breaks when the object changes between saving and loading
return group ? s16(group) : s16(-1);
});
std::copy(b.group_rotate.begin(), b.group_rotate.end(), group_rotate);
std::copy(b.group_rotate.begin(), b.group_rotate.begin() + SAVED_MAX_EXTRA_ROTATE, group_rotate);
#ifdef ARX_DEBUG
for(size_t i = SAVED_MAX_EXTRA_ROTATE; i < MAX_EXTRA_ROTATE; i++) {
arx_assert(!b.group_number[i]);
Expand Down

0 comments on commit 6a970e0

Please sign in to comment.