[Scummvm-tracker] [ScummVM :: Bugs] #15945: NANCY: nancy7: Heap overrun in RippedLetterPuzzle
ScummVM :: Bugs
trac at scummvm.org
Sat May 24 15:19:16 UTC 2025
#15945: NANCY: nancy7: Heap overrun in RippedLetterPuzzle
-------------------------+-------------------------------------------------
Reporter: | Owner: (none)
tunnelsociety |
Type: defect | Status: new
Priority: high | Component: Engine: Nancy
Version: | Resolution:
Keywords: | Game: Nancy Drew: Ghost Dogs of Moon
| Lake
-------------------------+-------------------------------------------------
Description changed by tunnelsociety:
Old description:
> A heap overrun occurs in Ghost Dogs of Moon Lake. Happens when trying to
> save the game after attempting certain puzzles. For example, load scene
> 3415, the Combo Cola puzzle, then try to leave Em's Emporium [which
> triggers a Second Chance save]. (Equally affects Roman Numerals puzzle at
> the ranger station.)
>
> These puzzles have fewer pieces than the RippedLetterPuzzle in nancy2
> STFD but the save code assumes a size.
>
> Arigatou ASan:
> {{{
> =================================================================
> ==13509==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x603000173184 at pc 0x560144813b39 bp 0x7ffec6b35bf0 sp 0x7ffec6b35be8
> READ of size 1 at 0x603000173184 thread T0
> #0 0x560144813b38 in void Common::Serializer::syncAsByte<signed
> char>(signed char&, unsigned int, unsigned int) common/serializer.h:117:2
> #1 0x5601448108ea in void Common::Serializer::syncArray<signed
> char>(signed char*, unsigned long, void (*)(Common::Serializer&, signed
> char&), unsigned int, unsigned int) common/serializer.h:297:4
> #2 0x56014480be23 in
> Nancy::RippedLetterPuzzleData::synchronize(Common::Serializer&)
> engines/nancy/puzzledata.cpp:76:6
> #3 0x5601443bdc37 in
> Nancy::State::Scene::synchronize(Common::Serializer&)
> engines/nancy/state/scene.cpp:737:15
> #4 0x56014474b4a4 in
> Nancy::NancyEngine::synchronize(Common::Serializer&)
> engines/nancy/nancy.cpp:662:18
> #5 0x56014474ba6d in
> Nancy::NancyEngine::saveGameStream(Common::WriteStream*, bool)
> engines/nancy/nancy.cpp:119:9
> #6 0x560144891f39 in Engine::saveGameState(int, Common::String
> const&, bool) engines/engine.cpp:924:25
> #7 0x56014474c404 in Nancy::NancyEngine::secondChance()
> engines/nancy/nancy.cpp:136:2
> #8 0x5601444e15bc in Nancy::Action::SaveContinueGame::execute()
> engines/nancy/action/miscrecords.cpp:163:11
> #9 0x56014446f25a in
> Nancy::Action::ActionManager::processActionRecords()
> engines/nancy/action/actionmanager.cpp:261:12
> #10 0x5601443ab8a4 in Nancy::State::Scene::run()
> engines/nancy/state/scene.cpp:1023:17
> #11 0x5601447579b1 in Nancy::NancyEngine::run()
> engines/nancy/nancy.cpp:318:7
> ...
>
> 0x603000173184 is located 0 bytes to the right of 20-byte region
> [0x603000173170,0x603000173184)
> allocated by thread T0 here:
> #0 0x5601442cf07e in malloc (scummvm+0xec207e) (BuildId:
> c37659539ee7296c)
> #1 0x5601445de28d in Common::Array<signed
> char>::allocCapacity(unsigned int) common/array.h:470:20
> #2 0x5601445fd252 in Common::Array<signed
> char>::operator=(Common::Array<signed char> const&) common/array.h:291:3
> #3 0x5601445f195c in Nancy::Action::RippedLetterPuzzle::execute()
> engines/nancy/action/puzzle/rippedletterpuzzle.cpp:206:24
> #4 0x56014446f25a in
> Nancy::Action::ActionManager::processActionRecords()
> engines/nancy/action/actionmanager.cpp:261:12
> #5 0x5601443ab8a4 in Nancy::State::Scene::run()
> engines/nancy/state/scene.cpp:1023:17
> #6 0x5601447579b1 in Nancy::NancyEngine::run()
> engines/nancy/nancy.cpp:318:7
> ...
>
> SUMMARY: AddressSanitizer: heap-buffer-overflow common/serializer.h:117:2
> in void Common::Serializer::syncAsByte<signed char>(signed char&,
> unsigned int, unsigned int)
> Shadow bytes around the buggy address:
> 0x0c06800265e0: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fd
> 0x0c06800265f0: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
> 0x0c0680026600: fd fa fa fa fd fd fd fa fa fa fd fd fd fd fa fa
> 0x0c0680026610: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd
> 0x0c0680026620: fa fa fd fd fd fd fa fa fd fd fd fd fa fa 00 00
> =>0x0c0680026630:[04]fa fa fa 00 00 04 fa fa fa fd fd fd fd fa fa
> 0x0c0680026640: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd
> 0x0c0680026650: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd
> 0x0c0680026660: fd fd fa fa fd fd fd fd fa fa fd fd fd fd fa fa
> 0x0c0680026670: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd
> 0x0c0680026680: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> ==13509==ABORTING
> }}}
>
> My probably unacceptable hotfix to keep playing on my build was:
> {{{#!diff
> diff --git a/engines/nancy/puzzledata.cpp b/engines/nancy/puzzledata.cpp
> index b17fe76a763..2a18862b4a1 100644
> --- a/engines/nancy/puzzledata.cpp
> +++ b/engines/nancy/puzzledata.cpp
> @@ -58,9 +58,11 @@ RippedLetterPuzzleData::RippedLetterPuzzleData() :
> playerHasTriedPuzzle(false) {}
>
> void RippedLetterPuzzleData::synchronize(Common::Serializer &ser) {
> + const int numPieces = g_nancy->getGameType() == kGameTypeNancy7 ?
> 20 : 24;
> +
> if (ser.isLoading()) {
> - order.resize(24);
> - rotations.resize(24);
> + order.resize(numPieces);
> + rotations.resize(numPieces);
> } else {
> // A piece may still be held while saving; make sure the
> saved data
> // has it back in the last place it was picked up from
> @@ -73,8 +75,8 @@ void
> RippedLetterPuzzleData::synchronize(Common::Serializer &ser) {
> }
> }
>
> - ser.syncArray(order.data(), 24, Common::Serializer::Byte);
> - ser.syncArray(rotations.data(), 24, Common::Serializer::Byte);
> + ser.syncArray(order.data(), numPieces, Common::Serializer::Byte);
> + ser.syncArray(rotations.data(), numPieces,
> Common::Serializer::Byte);
> }
>
> TowerPuzzleData::TowerPuzzleData() {
> ```
New description:
A heap overrun occurs in Ghost Dogs of Moon Lake. Happens when trying to
save the game after attempting certain puzzles. For example, load scene
3415, the Combo Cola puzzle, then try to leave Em's Emporium [which
triggers a Second Chance save]. (Equally affects Roman Numerals puzzle at
the ranger station.)
These puzzles have fewer pieces than the RippedLetterPuzzle in nancy2 STFD
but the save code assumes a size.
Arigatou ASan:
{{{
=================================================================
==13509==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x603000173184 at pc 0x560144813b39 bp 0x7ffec6b35bf0 sp 0x7ffec6b35be8
READ of size 1 at 0x603000173184 thread T0
#0 0x560144813b38 in void Common::Serializer::syncAsByte<signed
char>(signed char&, unsigned int, unsigned int) common/serializer.h:117:2
#1 0x5601448108ea in void Common::Serializer::syncArray<signed
char>(signed char*, unsigned long, void (*)(Common::Serializer&, signed
char&), unsigned int, unsigned int) common/serializer.h:297:4
#2 0x56014480be23 in
Nancy::RippedLetterPuzzleData::synchronize(Common::Serializer&)
engines/nancy/puzzledata.cpp:76:6
#3 0x5601443bdc37 in
Nancy::State::Scene::synchronize(Common::Serializer&)
engines/nancy/state/scene.cpp:737:15
#4 0x56014474b4a4 in
Nancy::NancyEngine::synchronize(Common::Serializer&)
engines/nancy/nancy.cpp:662:18
#5 0x56014474ba6d in
Nancy::NancyEngine::saveGameStream(Common::WriteStream*, bool)
engines/nancy/nancy.cpp:119:9
#6 0x560144891f39 in Engine::saveGameState(int, Common::String const&,
bool) engines/engine.cpp:924:25
#7 0x56014474c404 in Nancy::NancyEngine::secondChance()
engines/nancy/nancy.cpp:136:2
#8 0x5601444e15bc in Nancy::Action::SaveContinueGame::execute()
engines/nancy/action/miscrecords.cpp:163:11
#9 0x56014446f25a in
Nancy::Action::ActionManager::processActionRecords()
engines/nancy/action/actionmanager.cpp:261:12
#10 0x5601443ab8a4 in Nancy::State::Scene::run()
engines/nancy/state/scene.cpp:1023:17
#11 0x5601447579b1 in Nancy::NancyEngine::run()
engines/nancy/nancy.cpp:318:7
...
0x603000173184 is located 0 bytes to the right of 20-byte region
[0x603000173170,0x603000173184)
allocated by thread T0 here:
#0 0x5601442cf07e in malloc (scummvm+0xec207e) (BuildId:
c37659539ee7296c)
#1 0x5601445de28d in Common::Array<signed
char>::allocCapacity(unsigned int) common/array.h:470:20
#2 0x5601445fd252 in Common::Array<signed
char>::operator=(Common::Array<signed char> const&) common/array.h:291:3
#3 0x5601445f195c in Nancy::Action::RippedLetterPuzzle::execute()
engines/nancy/action/puzzle/rippedletterpuzzle.cpp:206:24
#4 0x56014446f25a in
Nancy::Action::ActionManager::processActionRecords()
engines/nancy/action/actionmanager.cpp:261:12
#5 0x5601443ab8a4 in Nancy::State::Scene::run()
engines/nancy/state/scene.cpp:1023:17
#6 0x5601447579b1 in Nancy::NancyEngine::run()
engines/nancy/nancy.cpp:318:7
...
SUMMARY: AddressSanitizer: heap-buffer-overflow common/serializer.h:117:2
in void Common::Serializer::syncAsByte<signed char>(signed char&, unsigned
int, unsigned int)
Shadow bytes around the buggy address:
0x0c06800265e0: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fd
0x0c06800265f0: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
0x0c0680026600: fd fa fa fa fd fd fd fa fa fa fd fd fd fd fa fa
0x0c0680026610: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd
0x0c0680026620: fa fa fd fd fd fd fa fa fd fd fd fd fa fa 00 00
=>0x0c0680026630:[04]fa fa fa 00 00 04 fa fa fa fd fd fd fd fa fa
0x0c0680026640: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd
0x0c0680026650: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd
0x0c0680026660: fd fd fa fa fd fd fd fd fa fa fd fd fd fd fa fa
0x0c0680026670: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd
0x0c0680026680: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==13509==ABORTING
}}}
My probably unacceptable patch to keep playing on my build was:
{{{#!diff
diff --git a/engines/nancy/puzzledata.cpp b/engines/nancy/puzzledata.cpp
index b17fe76a763..2a18862b4a1 100644
--- a/engines/nancy/puzzledata.cpp
+++ b/engines/nancy/puzzledata.cpp
@@ -58,9 +58,11 @@ RippedLetterPuzzleData::RippedLetterPuzzleData() :
playerHasTriedPuzzle(false) {}
void RippedLetterPuzzleData::synchronize(Common::Serializer &ser) {
+ const int numPieces = g_nancy->getGameType() == kGameTypeNancy7 ?
20 : 24;
+
if (ser.isLoading()) {
- order.resize(24);
- rotations.resize(24);
+ order.resize(numPieces);
+ rotations.resize(numPieces);
} else {
// A piece may still be held while saving; make sure the
saved data
// has it back in the last place it was picked up from
@@ -73,8 +75,8 @@ void
RippedLetterPuzzleData::synchronize(Common::Serializer &ser) {
}
}
- ser.syncArray(order.data(), 24, Common::Serializer::Byte);
- ser.syncArray(rotations.data(), 24, Common::Serializer::Byte);
+ ser.syncArray(order.data(), numPieces, Common::Serializer::Byte);
+ ser.syncArray(rotations.data(), numPieces,
Common::Serializer::Byte);
}
TowerPuzzleData::TowerPuzzleData() {
```
--
--
Ticket URL: <https://bugs.scummvm.org/ticket/15945#comment:1>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM
More information about the Scummvm-tracker
mailing list