[Scummvm-tracker] [ScummVM :: Bugs] #15884: SCUMM: INDY4: ASan o5_stringOps() heap buffer-overflow in copy-protection screen

[ScummVM :: Bugs]

#15884: SCUMM: INDY4: ASan o5_stringOps() heap buffer-overflow in copy-protection
screen
---------------------+------------------------------
Reporter:  dwatteau  |       Owner:  (none)
    Type:  defect    |      Status:  new
Priority:  normal    |   Component:  Engine: SCUMM
 Version:            |  Resolution:
Keywords:            |        Game:  Indiana Jones 4
---------------------+------------------------------
Comment (by eriktorbjorn):

 Though on second thought, rather than a game-specific fix we could simply
 return 0 on reading outside of the string (since we know it happens at
 least here). And maybe ignore attempts to write outside the string while
 we're at it.

 For this particular bug, returning 0 should still go to the default "do
 nothing" case, same as returning "0".

 Something like this (untested), perhaps?

 {{{
 diff --git a/engines/scumm/script_v5.cpp b/engines/scumm/script_v5.cpp
 index f28bfa0b5e4..a397555a6c8 100644
 --- a/engines/scumm/script_v5.cpp
 +++ b/engines/scumm/script_v5.cpp
 @@ -3052,6 +3052,7 @@ void ScummEngine_v5::o5_stopScript() {
  void ScummEngine_v5::o5_stringOps() {
         int a, b, c, i;
         byte *ptr;
 +       int len;

         _opcode = fetchScriptByte();
         switch (_opcode & 0x1F) {
 @@ -3074,9 +3075,11 @@ void ScummEngine_v5::o5_stringOps() {
                 b = getVarOrDirectByte(PARAM_2);
                 c = getVarOrDirectByte(PARAM_3);
                 ptr = getResourceAddress(rtString, a);
 +               len = getResourceSize(rtString, a);
                 if (ptr == nullptr)
                         error("String %d does not exist", a);
 -               ptr[b] = c;
 +               if (b >= 0 && b < len)
 +                       ptr[b] = c;
                 break;

         case 4:
 /* get string char */
 @@ -3084,9 +3087,13 @@ void ScummEngine_v5::o5_stringOps() {
                 a = getVarOrDirectByte(PARAM_1);
                 b = getVarOrDirectByte(PARAM_2);
                 ptr = getResourceAddress(rtString, a);
 +               len = getResourceSize(rtString, a);
                 if (ptr == nullptr)
                         error("String %d does not exist", a);
 -               setResult(ptr[b]);
 +               if (b >= 0 && b < len)
 +                       setResult(ptr[b]);
 +               else
 +                       setResult(0);
                 break;

         case 5:
 /* create empty string */
 }}}
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/15884#comment:4>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM