[Gnash-commit] /srv/bzr/gnash/trunk r11145: Handle out-of-bounds pool access, which might conceivably happen with

[Benjamin Wolsey]

------------------------------------------------------------
revno: 11145
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Thu 2009-06-18 08:11:06 +0200
message:
  Handle out-of-bounds pool access, which might conceivably happen with 
  malformed SWFs as well because of Gnash bugs.
modified:
  libcore/parser/abc_block.h
  libcore/vm/Machine.cpp
    ------------------------------------------------------------
    revno: 11134.1.1
    committer: Benjamin Wolsey <address@hidden>
    branch nick: test
    timestamp: Wed 2009-06-17 11:04:48 +0200
    message:
      Check function exists before calling it.
    modified:
      libcore/vm/Machine.cpp
    ------------------------------------------------------------
    revno: 11134.1.2
    committer: Benjamin Wolsey <address@hidden>
    branch nick: test
    timestamp: Wed 2009-06-17 11:26:05 +0200
    message:
      Throw an ASException when the pool is accessed out of range.
    modified:
      libcore/parser/abc_block.h
      libcore/vm/Machine.cpp
    ------------------------------------------------------------
    revno: 11134.1.3
    committer: Benjamin Wolsey <address@hidden>
    branch nick: test
    timestamp: Wed 2009-06-17 11:36:39 +0200
    message:
      Minor cleanups.
    modified:
      libcore/vm/Machine.cpp

=== modified file 'libcore/parser/abc_block.h'
--- a/libcore/parser/abc_block.h        2009-05-14 11:23:13 +0000
+++ b/libcore/parser/abc_block.h        2009-06-17 09:26:05 +0000
@@ -29,6 +29,7 @@
 #include <vector>
 #include <string>
 #include <boost/scoped_array.hpp>
+#include <stdexcept>
 
 namespace gnash {
        class SWFStream; // for read signature
@@ -120,7 +121,18 @@
 
 } // namespace abc
 
-                       
+namespace {
+
+template<typename T>
+inline void checkBounds(size_t i, const T& container)
+{
+    if (i >= container.size()) {
+        throw std::range_error("Attempt to access pool out of range");
+    }
+}
+
+}
+
 class abc_block
 {
 public:
@@ -151,42 +163,42 @@
     }
 
     boost::uint32_t uIntegerPoolAt(size_t i) const {
-        assert(i < _uIntegerPool.size());
+        checkBounds(i, _uIntegerPool);
         return _uIntegerPool[i];
     }
 
     const std::string& stringPoolAt(size_t i) const {
-        assert(i < _stringPool.size());
+        checkBounds(i, _stringPool);
         return _stringPool[i];
     }
 
     boost::int32_t integerPoolAt(size_t i) const {
-        assert(i < _integerPool.size());
+        checkBounds(i, _integerPool);
         return _integerPool[i];
     }
 
     double doublePoolAt(size_t i) const {
-        assert(i < _doublePool.size());
+        checkBounds(i, _doublePool);
         return _doublePool[i];
     }
 
     asMethod* methodPoolAt(size_t i) const {
-        assert(i < _methods.size());
+        checkBounds(i, _methods);
         return _methods[i];
     }
 
     asName multinamePoolAt(size_t i) const {
-        assert(i < _multinamePool.size());
+        checkBounds(i, _multinamePool);
         return _multinamePool[i];
     }
 
     asClass* classPoolAt(size_t i) const {
-        assert(i < _classes.size());
+        checkBounds(i, _classes);
         return _classes[i];
     }
 
     asNamespace* namespacePoolAt(size_t i) const {
-        assert(i < _namespacePool.size());
+        checkBounds(i, _namespacePool);
         return _namespacePool[i];
     }
 

=== modified file 'libcore/vm/Machine.cpp'
--- a/libcore/vm/Machine.cpp    2009-06-16 13:21:33 +0000
+++ b/libcore/vm/Machine.cpp    2009-06-17 09:36:39 +0000
@@ -54,73 +54,114 @@
        {}
 };
 
-// Functions for getting pool constants.
-static inline const std::string&
+/// Functions for getting pool constants.
+//
+/// TODO: it's quite possible for a malformed SWF to ask for out-of-bounds
+/// pool access, although at the moment it's mainly Gnash bugs causing this.
+/// Throwing an exception is good here, but it's not clear which one.
+namespace {
+
+inline const std::string&
 pool_string(boost::uint32_t index, abc_block *pool)
 {
        if (!pool) throw ASException();
-       return pool->stringPoolAt(index);
+    try {
+        return pool->stringPoolAt(index);
+    }
+    catch (std::range_error& e) {
+        throw ASException();
+    }
 }
 
-static inline int
+inline int
 pool_int(boost::uint32_t index, abc_block *pool)
 {
        if (!pool) throw ASException();
-       return pool->integerPoolAt(index);
+    try {
+        return pool->integerPoolAt(index);
+    }
+    catch (std::range_error& e) {
+        throw ASException();
+    }
 }
 
-static inline unsigned int
+inline unsigned int
 pool_uint(boost::uint32_t index, abc_block *pool)
 {
        if (!pool) throw ASException();
-       return pool->uIntegerPoolAt(index);
+    try {
+        return pool->uIntegerPoolAt(index);
+    }
+    catch (std::range_error& e) {
+        throw ASException();
+    }
 }
 
-static inline double
+inline double
 pool_double(boost::uint32_t index, abc_block *pool)
 {
        if (!pool) throw ASException();
-       log_abc("Getting double from pool at index %u",index);
-       return pool->doublePoolAt(index);
+    try {
+        return pool->doublePoolAt(index);
+    }
+    catch (std::range_error& e) {
+        throw ASException();
+    }
 }
 
-static inline asNamespace*
+inline asNamespace*
 pool_namespace(boost::uint32_t index, abc_block *pool)
 {
        if (!pool) throw ASException();
-       return pool->namespacePoolAt(index);
+    try {
+        return pool->namespacePoolAt(index);
+    }
+    catch (std::range_error& e) {
+        throw ASException();
+    }
+
 }
 
-static inline asMethod*
+inline asMethod*
 pool_method(boost::uint32_t index, abc_block* pool)
 {
        if (!pool) throw ASException();
-       return pool->methodPoolAt(index);
+    try {
+        return pool->methodPoolAt(index);
+    }
+    catch (std::range_error& e) {
+        throw ASException();
+    }
 }
 
-static inline asClass*
+inline asClass*
 pool_class(boost::uint32_t index, abc_block* pool)
 {
        if (!pool) throw ASException();
-       return pool->classPoolAt(index);
+    try {
+        return pool->classPoolAt(index);
+    }
+    catch (std::range_error& e) {
+        throw ASException();
+    }
 }
 
 // Don't make this a reference or you'll taint the pool.
-static inline asName
+inline asName
 pool_name(boost::uint32_t index, abc_block* pool)
 {
        if (!pool) throw ASException();
-       asName multiname = pool->multinamePoolAt(index);
-#if 0
-    log_abc("Searching multiname pool for property id=%u abc name=%u "
-            "global name = %u abc string=%s flags=0x%X name_space=%u",
-            index, multiname.getABCName(), multiname.getGlobalName(),
-            pool->mStringPool[multiname.getABCName()],multiname.mFlags | 0x0,
-            multiname.getNamespace()->getURI());
-#endif
-       return multiname;
+       try {
+        asName multiname = pool->multinamePoolAt(index);
+        return multiname;
+    }
+    catch (std::range_error& e) {
+        throw ASException();
+    }
 }
 
+} // anonymous namespace
+
 /// ENSURE_NUMBER makes sure that the given argument is a number,
 /// calling the valueOf method if necessary -- it's a macro so that
 /// the valueOf method may be pushed if needed, and then whatever
@@ -1198,6 +1239,10 @@
                 {
                     boost::uint32_t argc = mStream->read_V32();
                     as_function *f = mStack.top(argc).to_as_function();
+                    if (!f) {
+                        log_abc("CONSTRUCT: No function on stack!");
+                        break;
+                    }
                     Property b(0, 0, f, NULL);
                     pushCall(f, NULL, mStack.top(argc), argc, 0);
                     break;
@@ -3148,9 +3193,9 @@
 
        as_object *target = 0;
        as_environment env = as_environment(_vm);
-       std::string name = mPoolObject->stringPoolAt(multiname.getABCName());
-       std::string ns = mPoolObject->stringPoolAt(
-            multiname.getNamespace()->getAbcURI());
+       std::string name = pool_string(multiname.getABCName(), mPoolObject);
+       std::string ns = pool_string(multiname.getNamespace()->getAbcURI(),
+            mPoolObject);
        std::string path = ns.empty() ? name : ns + "." + name;
 
     log_abc("Failed to find property in scope stack. Looking for %s in "
@@ -3175,9 +3220,9 @@
         asName multiname)
 {
 
-       std::string ns = 
-        mPoolObject->stringPoolAt(multiname.getNamespace()->getAbcURI());
-       std::string name = mPoolObject->stringPoolAt(multiname.getABCName());
+       std::string ns = pool_string(multiname.getNamespace()->getAbcURI(),
+            mPoolObject);
+       std::string name = pool_string(multiname.getABCName(), mPoolObject);
        return get_property_value(obj, name, ns);
 }