Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-81037

Array.concat does not verify length against allocated space

    XMLWordPrintable

    Details

    • Commits:
      6fa617524a6d0a2bc988e2dc70e8d719d1b9c282 (qt/qtdeclarative/5.14)

      Description

      In QV4, we can update the length of an ArrayObject without actually changing the amount of memory allocated for the items in the array. This is reasonable, because otherwise array.length = 2000000 eats up a lot of memory for lots of undefined items.

      Unfortunately, Array.concat does not appear to verify that all the items that it copies are actually allocated — meaning that setting the length of an array in this manner and then using Array.concat allows an out-of-bounds read:

      function oob() {
          const v4 = [1,2,3];
          const v7 = [4,5];
          v7.length = 1337;
          const v9 = v4.concat(v7);
          gc(); // Crashes (tries to free invalid objects copied by Array.concat)
      }
      

      This shows how this bug can be used to obtain native code execution in qmljs from JavaScript running in QV4 (in short, it allows creating a fake QV4::Object from an address stored in a double by spraying these doubles across the QV4 heap using NumberObject, and also allows a use-after-free by obtaining references to freed objects using the out-of-bounds read, which allows reading the address of an arbitrary object, which ultimately leads to arbitrary read-write and code execution).

      (I found this bug while fuzzing with a modified version of Fuzzilli, in case anyone wants to go looking deeper into these types of bugs, or test against other versions; I limited myself to 5.13.2 because that was the current release on most systems when I went looking for this)

      I think that this happens because we trust getLength in qv4arrayobject.cpp:417, and do not check for this scenario in ArrayData::append (qv4arraydata.cpp:589). The length is forwarded through Object::arrayPut (qv4object.cpp:257) to SimpleArrayData::putArray (qv4arraydata.cpp:321), where it simply copies n items from the values pointer that is passed in. Of course, at this point we no longer know how much space was allocated. Maybe the best way to fix this would be to check that no more than os->values.alloc items are passed into arrayPut?

      This appears to be related to QTBUG-51581 (which addressed similar symptoms but does not appear to fix this issue).

        Attachments

          Issue Links

          For Gerrit Dashboard: QTBUG-81037
          # Subject Branch Project Status CR V

            Activity

              People

              Assignee:
              qt.team.quick.subscriptions Qt Quick and Widgets Team
              Reporter:
              hlt Tobias Holl
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Gerrit Reviews

                  There are no open Gerrit changes