From 6a970e07983d47d3b4a4fdea6431d99c03aefff0 Mon Sep 17 00:00:00 2001 From: Daniel Scharrer Date: Thu, 5 Jan 2023 20:10:06 +0100 Subject: [PATCH] ChangeLevel: Fix out of bounds access to NPC extra_rotate data in saves 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 --- src/scene/ChangeLevel.cpp | 6 +++++- src/scene/SaveFormat.h | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/scene/ChangeLevel.cpp b/src/scene/ChangeLevel.cpp index fa8ca2097a..6dd5acce9e 100644 --- a/src/scene/ChangeLevel.cpp +++ b/src/scene/ChangeLevel.cpp @@ -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()) { @@ -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)); diff --git a/src/scene/SaveFormat.h b/src/scene/SaveFormat.h index e5ba43cf6a..3612ee6562 100644 --- a/src/scene/SaveFormat.h +++ b/src/scene/SaveFormat.h @@ -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]);