[Gnash-commit] /srv/bzr/gnash/trunk r9861: Fix malformed SWF abort.

[Benjamin Wolsey]

------------------------------------------------------------
revno: 9861
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Mon 2008-09-29 15:35:56 +0200
message:
  Fix malformed SWF abort.
modified:
  libcore/parser/action_buffer.cpp
  libcore/parser/action_buffer.h
    ------------------------------------------------------------
    revno: 9860.1.1
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Mon 2008-09-29 15:34:41 +0200
    message:
      Throw parser exception if the constant pool dictionary is the wrong size,
      fixing an abort for a malformed SWF. Also drop unused methods.
    modified:
      libcore/parser/action_buffer.cpp
      libcore/parser/action_buffer.h

=== modified file 'libcore/parser/action_buffer.cpp'
--- a/libcore/parser/action_buffer.cpp  2008-09-03 15:22:47 +0000
+++ b/libcore/parser/action_buffer.cpp  2008-09-29 13:34:41 +0000
@@ -38,10 +38,6 @@
 using std::string;
 using std::endl;
 
-namespace {
-//gnash::LogFile& dbglogfile = gnash::LogFile::getDefaultInstance();
-}
-
 namespace gnash {
 
 // Forward declarations
@@ -53,8 +49,6 @@
     m_decl_dict_processed_at(-1),
     _src(md)
 {
-//    static int count=0;
-//    printf("Action buffer %d created\n", ++count);
 }
 
 void
@@ -117,23 +111,19 @@
 {
     assert(stop_pc <= m_buffer.size());
     
+
+    // Skip if we've already processed this decl_dict, but make sure
+    // the size is the same.
     if (static_cast<size_t>(m_decl_dict_processed_at) == start_pc) {
-    // We've already processed this decl_dict. 
-#ifndef NDEBUG
-    int count = read_int16(start_pc+3);
-    assert((int) m_dictionary.size() == count);
-#endif
-    return;
-    }
-    
-#if 0 // debugging
-    if (m_decl_dict_processed_at != -1)    {
-    log_debug(_("process_decl_dict(%d, %d): decl_dict was already processed at 
%d. "
-        "Overriding."),
-          start_pc, stop_pc, m_decl_dict_processed_at);
-    //return;
-    }
-#endif
+        const int dictSize = read_int16(start_pc + 3);
+        if (static_cast<int>(m_dictionary.size()) != dictSize)
+        {
+            /// TODO: is it possible to continue?
+            throw ActionParserException(_("Constant pool size "
+                        "mismatch. This is probably a very malformed SWF"));
+        }
+        return;
+    }
     
     m_decl_dict_processed_at = start_pc;
     
@@ -143,74 +133,32 @@
     boost::uint16_t count = boost::uint16_t(read_int16(i+3)); 
     i += 2;
     
-//log_debug(_("Start at %d, stop at %d, length read was %d, count read was 
%d"), start_pc, stop_pc, length, count);
-
     assert(start_pc + 3 + length == stop_pc);
     
     m_dictionary.resize(count);
     
     // Index the strings.
     for (int ct = 0; ct < count; ct++) {
-    // Point into the current action buffer.
-    m_dictionary[ct] = (const char*) &m_buffer[3 + i];
-    
-    while (m_buffer[3 + i]) {
-        // safety check.
-        if (i >= stop_pc) {
-        log_error(_("action buffer dict length exceeded"));
-        
-        // Jam something into the remaining (invalid) entries.
-        while (ct < count) {
-            m_dictionary[ct] = "<invalid>";
-            ct++;
-        }
-        return;
+        // Point into the current action buffer.
+        m_dictionary[ct] = (const char*) &m_buffer[3 + i];
+
+        while (m_buffer[3 + i]) {
+            // safety check.
+            if (i >= stop_pc) {
+                log_error(_("action buffer dict length exceeded"));
+                // Jam something into the remaining (invalid) entries.
+                while (ct < count) {
+                    m_dictionary[ct] = "<invalid>";
+                    ct++;
+                }
+            return;
+            }
+            i++;
         }
         i++;
     }
-    i++;
-    }
-}
-
-#if 0
-// Interpret the actions in this action buffer, and evaluate
-// them in the given environment.  Execute our whole buffer,
-// without any arguments passed in.
-void
-action_buffer::execute(as_environment* env) const
-{
-    assert(env);
-
-    int local_stack_top = env->get_local_frame_top();
-    env->add_frame_barrier();
-
-    ActionExec exec(*this, *env);
-    exec();
-    
-    env->set_local_frame_top(local_stack_top);
-}
-
-// Interpret the specified subset of the actions in our
-// buffer.  Caller is responsible for cleaning up our local
-// stack frame (it may have passed its arguments in via the
-// local stack frame).
-// 
-// The is_function2 flag determines whether to use global or local registers.
-void
-action_buffer::execute(
-    as_environment* env,
-    size_t start_pc,
-    size_t exec_bytes, // used when invoked as a function call
-    as_value* retval, // used when invoked as a function call
-    const std::vector<with_stack_entry>& initial_with_stack,
-    bool is_function2) const
-{
-    assert(env);
-    ActionExec exec(*this, *env, start_pc, exec_bytes, retval,
-        initial_with_stack, is_function2);
-    exec();
-}
-#endif
+}
+
 
 // Disassemble one instruction to the log. The maxBufferLength
 // argument is the number of bytes remaining in the action_buffer
@@ -220,9 +168,9 @@
 disasm_instruction(const unsigned char* instruction_data, size_t 
maxBufferLength)
 {
 
-    using namespace gnash::SWF;
+    using namespace SWF;
 
-    const gnash::SWF::SWFHandlers& ash = gnash::SWF::SWFHandlers::instance();
+    const SWF::SWFHandlers& ash = SWF::SWFHandlers::instance();
 
     assert (maxBufferLength > 0);
 

=== modified file 'libcore/parser/action_buffer.h'
--- a/libcore/parser/action_buffer.h    2008-09-04 15:32:42 +0000
+++ b/libcore/parser/action_buffer.h    2008-09-29 13:34:41 +0000
@@ -18,8 +18,7 @@
 #ifndef GNASH_ACTION_BUFFER_H
 #define GNASH_ACTION_BUFFER_H
 
-#include "types.h"
-
+#include <boost/noncopyable.hpp>
 #include <boost/cstdint.hpp> // for boost::uint8_t
 #include <vector> // for composition
 
@@ -47,7 +46,8 @@
 /// so to eventually use a gnash::stream directly and
 /// avoid full loads. (not before profiling!).
 //
-class action_buffer
+/// Good, would make jumping to other tags possible.
+class action_buffer : boost::noncopyable
 {
 public:
        friend class ActionExec;
@@ -145,18 +145,6 @@
                return reinterpret_cast<const unsigned char*>(&m_buffer.at(pc));
        }
 
-    /// Get the base pointer of the code buffer.
-    const unsigned char* getCodeStart()
-       {
-               return reinterpret_cast<const unsigned char*>(&m_buffer);
-       }
-
-       const unsigned char* get_buffer(size_t pc) const
-       {
-               assert(pc < m_buffer.size() );
-               return reinterpret_cast<const unsigned char*>(&m_buffer[pc]);
-       }
-
        /// Get a signed integer value from given offset
        //
        /// Useful to hide complexity of underlying buffer access.
@@ -248,12 +236,6 @@
 
 private:
 
-       // Don't put these as values in std::vector<>!  They contain
-       // internal pointers and cannot be moved or copied.
-       // If you need to keep an array of them, keep pointers
-       // to new'd instances.
-       action_buffer(const action_buffer& a);
-
        /// the code itself, as read from the SWF
        std::vector<boost::uint8_t> m_buffer;