[Scummvm-tracker] [ScummVM :: Bugs] #14578: SHERLOCK: ROSETATTOO: Incorrect talk history due to out-of-bounds access

[ScummVM :: Bugs]

#14578: SHERLOCK: ROSETATTOO: Incorrect talk history due to out-of-bounds access
-------------------------+-------------------------------------------------
Reporter:  PushmePullyu  |       Owner:  PushmePullyu
    Type:  defect        |      Status:  pending
Priority:  normal        |   Component:  Engine: Sherlock
 Version:                |  Resolution:  pending
Keywords:                |        Game:  Sherlock Holmes: Case of the Rose
                         |  Tattoo
-------------------------+-------------------------------------------------
Description changed by PushmePullyu:

Old description:

> Tested with master !e0a146f9086ae523229d6e8173c39d290e669449 on Linux
> x86_64 using the English CD version of Rose Tattoo.
>
> The talk history is held in
>
> {{{
> Common::Array<Sherlock::TalkHistoryEntry> Sherlock::Talk::_talkHistory
> }}}
>
> Every TalkHistoryEntry holds 16 bools, each corresponding to a statement:
>
> engines/sherlock/talk.h:
> {{{
> struct TalkHistoryEntry {
>         bool _data[16];
>         ...
> }
> }}}
>

> However, some talk files have more than 16 statements. Since this is
> never checked, it will lead to out-of-bounds writes/reads, accessing
> memory in the next TalkHistoryEntry instead.
>
> Other than text color, incorrect talk history also affects recording of
> statements in the journal and updating the _holmesQuotient.
>

> To reproduce with "WIGG01B.TLK" statement 17
> (_talkHistory![984]._data![16]) and "SPIT36D.TLK" statement 1
> (_talkHistory![985]._data![0]):
> 1. Load attached save
> 2. Talk to Wiggins; note that option 3 is not marked as used before (it
> should be shown brighter than option 1)
> 3. Abort the talk without selecting any statement by pressing ESC
> 4. Exit to the city map
> 5. Go to Spitalfields (rightmost icon on the map)
> 6. Exit to the city map
> 7. Go back to 221B Baker St. (magnifying glass icon in the NW part of the
> city)
> 8. Talk to Wiggins again; note that option 3 is now marked as used (it
> should be shown darker)
>
> Notes:
> - Rose Tattoo's "WATS02A.TLK" contains 31 statements, which seems to be
> the largest number used.
> - I did not test this with Scalpel nor do I know its maximum number of
> statements.

New description:

 Tested with master !e0a146f9086ae523229d6e8173c39d290e669449 on Linux
 x86_64 using the English CD version of Rose Tattoo.

 The talk history is held in

 {{{
 Common::Array<Sherlock::TalkHistoryEntry> Sherlock::Talk::_talkHistory
 }}}

 Every TalkHistoryEntry holds 16 bools, each corresponding to a statement:

 engines/sherlock/talk.h:
 {{{
 struct TalkHistoryEntry {
         bool _data[16];
         ...
 }
 }}}


 However, some talk files have more than 16 statements. Since this is never
 checked, it will lead to out-of-bounds writes/reads, accessing memory in
 the next TalkHistoryEntry instead.

 Other than text color, incorrect talk history also affects recording of
 statements in the journal and updating the _holmesQuotient.


 To reproduce with "WIGG01B.TLK" statement 17
 (_talkHistory![984]._data![16]) and "SPIT36D.TLK" statement 1
 (_talkHistory![985]._data![0]):
 1. Load attached save
 2. Talk to Wiggins; note that option 3 is not marked as used before (it
 should be shown brighter than option 1)
 3. Abort the talk without selecting any statement by pressing ESC
 4. Exit to the city map
 5. Go to Spitalfields (rightmost icon on the map)
 6. Exit to the city map
 7. Go back to 221B Baker St. (magnifying glass icon in the NW part of the
 city)
 8. Talk to Wiggins again; note that option 3 is now marked as used (it
 should be shown darker)

 Notes:
 - Rose Tattoo's "WATS02A.TLK" contains 31 statements, which seems to be
 the largest number used.
 - ~~I did not test this with Scalpel nor do I know its maximum number of
 statements.~~
 - Scalpel's talk files never exceed 16 statements, so it is not affected
 by the problem

--
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/14578#comment:2>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM