[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Gnash-commit] gnash ChangeLog server/parser/action_buffer.cpp...
|
From: |
Benjamin Wolsey |
|
Subject: |
[Gnash-commit] gnash ChangeLog server/parser/action_buffer.cpp... |
|
Date: |
Tue, 17 Jun 2008 11:34:59 +0000 |
CVSROOT: /sources/gnash
Module name: gnash
Changes by: Benjamin Wolsey <bwy> 08/06/17 11:34:59
Modified files:
. : ChangeLog
server/parser : action_buffer.cpp
server/vm : ASHandlers.cpp
Log message:
* server/vm/action_buffer.cpp: rewrite disasm_instruction so
that it
doesn't read uninitialized memory so much (still not entirely
unbreakable). Use switch and hexify whole instructions where
the
length is known.
* server/vm/ASHandlers.cpp: fix output of uint8_t.
CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.6954&r2=1.6955
http://cvs.savannah.gnu.org/viewcvs/gnash/server/parser/action_buffer.cpp?cvsroot=gnash&r1=1.43&r2=1.44
http://cvs.savannah.gnu.org/viewcvs/gnash/server/vm/ASHandlers.cpp?cvsroot=gnash&r1=1.251&r2=1.252
Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.6954
retrieving revision 1.6955
diff -u -b -r1.6954 -r1.6955
--- ChangeLog 17 Jun 2008 09:28:27 -0000 1.6954
+++ ChangeLog 17 Jun 2008 11:34:58 -0000 1.6955
@@ -1,3 +1,11 @@
+2008-06-17 Benjamin Wolsey <address@hidden>
+
+ * server/vm/action_buffer.cpp: rewrite disasm_instruction so that it
+ doesn't read uninitialized memory so much (still not entirely
+ unbreakable). Use switch and hexify whole instructions where the
+ length is known.
+ * server/vm/ASHandlers.cpp: fix output of uint8_t.
+
2008-06-17 Sandro Santilli <address@hidden>
* server/: Makefile.am, as_value.{cpp,h}, ChracterProxy.{cpp,h}:
Index: server/parser/action_buffer.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/parser/action_buffer.cpp,v
retrieving revision 1.43
retrieving revision 1.44
diff -u -b -r1.43 -r1.44
--- server/parser/action_buffer.cpp 9 Jun 2008 18:12:57 -0000 1.43
+++ server/parser/action_buffer.cpp 17 Jun 2008 11:34:59 -0000 1.44
@@ -211,123 +211,160 @@
}
#endif
-// Disassemble one instruction to the log.
+// Disassemble one instruction to the log. The maxBufferLength
+// argument is the number of bytes remaining in the action_buffer
+// and prevents malformed instructions causing a read past the
+// end of the buffer.
static std::string
-disasm_instruction(const unsigned char* instruction_data)
+disasm_instruction(const unsigned char* instruction_data, size_t
maxBufferLength)
{
using namespace gnash::SWF;
const gnash::SWF::SWFHandlers& ash = gnash::SWF::SWFHandlers::instance();
+ assert (maxBufferLength > 0);
+
as_arg_t fmt = ARG_HEX;
- action_type action_id = (action_type)instruction_data[0];
- unsigned char num[10];
- memset(num, 0, 10);
+ action_type action_id = static_cast<action_type>(instruction_data[0]);
std::stringstream ss;
// Show instruction.
if (action_id > ash.lastType()) {
ss << "<unknown>[0x]" << action_id << endl;
- } else {
+ }
+ else {
ss << ash[action_id].getName();
}
// Show instruction argument(s).
if (action_id & 0x80)
{
+ assert (maxBufferLength >= 3);
ss << " (";
fmt = ash[action_id].getArgFormat();
assert(fmt != ARG_NONE);
- int length = instruction_data[1] | (instruction_data[2] << 8);
- if (fmt == ARG_HEX)
+ size_t length = (instruction_data[1] | (instruction_data[2] << 8));
+
+ // Assert that length without the three initial bytes
+ // is always within the buffer.
+ assert (length <= maxBufferLength - 3);
+
+ switch (fmt)
{
- for (int i = 0; i < length; i++) {
+ case ARG_NONE:
+ break;
- ss << "0x" << hexify((const boost::uint8_t
*)&instruction_data[3 + i], 1, false) << " ";
- }
- }
- else if (fmt == ARG_STR)
+ case ARG_HEX:
{
- string str;
- for (int i = 0; i < length; i++) {
- str += instruction_data[3 + i];
+ ss << hexify(&instruction_data[3], length, false) << " ";
+ break;
}
+
+ case ARG_STR:
+ {
+ std::string str = hexify(&instruction_data[3], length, true);
ss << "\"" << str.c_str() << "\"";
+ break;
}
- else if (fmt == ARG_U8)
+
+ case ARG_U8:
{
int val = instruction_data[3];
ss << " " << val;
+ break;
}
- else if (fmt == ARG_U16)
+
+ case ARG_U16:
{
int val = instruction_data[3] | (instruction_data[4] << 8);
ss << " " << val;
+ break;
}
- else if (fmt == ARG_S16)
+
+ case ARG_S16:
{
int val = instruction_data[3] | (instruction_data[4] << 8);
if (val & 0x8000) val |= ~0x7FFF; // sign-extend
ss << " " << val;
+ break;
}
- else if (fmt == ARG_PUSH_DATA)
+
+ case ARG_PUSH_DATA:
{
- int i = 0;
+ size_t i = 0;
while (i < length)
{
int type = instruction_data[3 + i];
+
+ // This should be safe, as the buffer is always
0-terminated.
if ( i++ ) ss << ", ";
- if (type == 0)
+ switch (type)
+ {
+
+ case 0:
{
// string
- string str;
- while (instruction_data[3 + i])
+ std::string str;
+ while (instruction_data[3 + i] && i < length)
{
- str += instruction_data[3 + i];
+ str += hexify(&instruction_data[3 + i], 1,
true);
i++;
}
i++;
ss << "\"" << str.c_str() << "\"";
+ break;
}
- else if (type == 1)
+
+ case 1:
{
// float (little-endian)
+ if (i + 4 > length) break;
float f = convert_float_little(instruction_data + 3 + i);
i += 4;
ss << "(float) " << f;
+ break;
}
- else if (type == 2)
- {
+
+ case 2:
ss << "NULL";
- }
- else if (type == 3)
- {
+ break;
+
+ case 3:
ss << "undef";
- }
- else if (type == 4)
+ break;
+
+ case 4:
{
// contents of register
int reg = instruction_data[3 + i];
i++;
ss << "reg[" << reg << "]";
+ break;
}
- else if (type == 5)
+
+ case 5:
{
+
int bool_val = instruction_data[3 + i];
i++;
ss << "bool(" << bool_val << ")";
+ break;
}
- else if (type == 6)
+
+ case 6:
{
// double in wacky format: 45670123
+ if (i + 8 > length) break;
double d = convert_double_wacky(instruction_data + 3 + i);
i += 8;
ss << "(double) " << d;
+ break;
}
- else if (type == 7)
+
+ case 7:
{
// boost::int32_t
boost::int32_t val = instruction_data[3 + i]
@@ -336,68 +373,80 @@
| (instruction_data[3 + i + 3] << 24);
i += 4;
ss << "(int) " << val;
+ break;
}
- else if (type == 8)
+
+ case 8:
{
int id = instruction_data[3 + i];
i++;
ss << "dict_lookup[" << id << "]";
+ break;
}
- else if (type == 9)
+
+ case 9:
{
int id = instruction_data[3 + i] | (instruction_data[3 + i
+ 1] << 8);
i += 2;
ss << "dict_lookup_lg[" << id << "]";
+ break;
}
}
}
- else if (fmt == ARG_DECL_DICT)
+ break;
+ }
+
+ case ARG_DECL_DICT:
{
- int i = 0;
- int count = instruction_data[3 + i] | (instruction_data[3 + i + 1]
<< 8);
+ size_t i = 0;
+ size_t count = instruction_data[3 + i] | (instruction_data[3 +
i + 1] << 8);
i += 2;
ss << " [" << count << "] ";
// Print strings.
- for (int ct = 0; ct < count; ct++)
+ for (size_t ct = 0; ct < count; ct++)
{
if ( ct ) ss << ", ";
ss << ct << ":";
- string str;
- while (instruction_data[3 + i])
+ std::string str;
+ while (instruction_data[3 + i] && i < length)
{
- // safety check.
- if (i >= length)
- {
- log_debug("<disasm error -- length exceeded>");
- break;
- }
str += instruction_data[3 + i];
i++;
}
ss << "\"" << str.c_str() << "\"";
i++;
}
+ break;
}
- else if (fmt == ARG_FUNCTION2)
+
+ case ARG_FUNCTION2:
{
+ size_t i = 0;
+ std::string functionName;
// Signature info for a function2 opcode.
- int i = 0;
- const char* function_name = (const char*) &instruction_data[3 + i];
- i += strlen(function_name) + 1;
+ while (instruction_data[3 + i] && i <= length)
+ {
+ functionName.push_back(instruction_data[3 + i]);
+ ++i;
+ }
+
+ // Don't read outside the instruction.
+ if (i + 6 > length) break;
+ ++i;
- int arg_count = instruction_data[3 + i] | (instruction_data[3 + i +
1] << 8);
+ boost::uint16_t argCount = instruction_data[3 + i] |
(instruction_data[3 + i + 1] << 8);
i += 2;
- int reg_count = instruction_data[3 + i];
+ boost::uint8_t registerCount = instruction_data[3 + i];
i++;
- ss << "\tname = '" << function_name << "'"
- << " arg_count = " << arg_count
- << " reg_count = " << reg_count;
+ ss << "\tname = '" << functionName << "'"
+ << " arg count = " << argCount
+ << " register count = " <<
static_cast<int>(registerCount);
boost::uint16_t flags = (instruction_data[3 + i]) |
(instruction_data[3 + i + 1] << 8);
i += 2;
@@ -424,25 +473,45 @@
<< " st=" << suppress_this
<< " pt=" << preload_this;
- for (int argi = 0; argi < arg_count; argi++)
+ for (size_t argi = 0; argi < argCount; ++argi)
{
+ // Make sure not to read past the end of the
+ // instruction.
+ if (i >= length) break;
+
int arg_register = instruction_data[3 + i];
i++;
- const char* arg_name = (const char*) &instruction_data[3 +
i];
- i += strlen(arg_name) + 1;
+
+ std::string argName;
+ // Signature info for a function2 opcode.
+ while (instruction_data[3 + i] && i <= length)
+ {
+ argName.push_back(instruction_data[3 + i]);
+ i++;
+ }
ss << "\targ[" << argi << "]"
<< " - reg[" << arg_register << "]"
- << " - '" << arg_name << "'";
+ << " - '" << argName << "'";
+
+ if (i == length) break;
+
+ // Advance past the terminating 0
+ i++;
+
}
+ if (i + 2 > length) break;
int function_length = instruction_data[3 + i] | (instruction_data[3
+ i + 1] << 8);
i += 2;
ss << "\t\tfunction length = " << function_length;
+ break;
}
+ } // Switch
+
ss << ")";
- }
+ } // If action & 0x80
return ss.str();
}
@@ -450,9 +519,8 @@
std::string
action_buffer::disasm(size_t pc) const
{
- const unsigned char* instruction_data =
- (const unsigned char *)&m_buffer[pc];
- return disasm_instruction(instruction_data);
+ const size_t maxBufferLength = m_buffer.size() - pc;
+ return disasm_instruction(&m_buffer[pc], maxBufferLength);
}
// Endian conversion routines.
Index: server/vm/ASHandlers.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/vm/ASHandlers.cpp,v
retrieving revision 1.251
retrieving revision 1.252
diff -u -b -r1.251 -r1.252
--- server/vm/ASHandlers.cpp 16 Jun 2008 12:23:06 -0000 1.251
+++ server/vm/ASHandlers.cpp 17 Jun 2008 11:34:59 -0000 1.252
@@ -4119,8 +4119,11 @@
thread.next_pc = i; // Proceed into the try block.
IF_VERBOSE_ACTION(
- log_action(_("ActionTry: reserved:%x doFinally:%d doCatch:%d trySize:%u
catchSize:%u finallySize:%u catchName:%s catchRegister:%u"),
- reserved, doFinally, doCatch, trySize, catchSize, finallySize,
catchName ? catchName : "(null)", catchRegister);
+ log_action(_("ActionTry: reserved:%x doFinally:%d doCatch:%d trySize:%u "
+ "catchSize:%u finallySize:%u catchName:%s catchRegister:%u"),
+ static_cast<int>(reserved), doFinally, doCatch, trySize,
+ catchSize, finallySize,
+ catchName ? catchName : "(null)", catchRegister);
);
}
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Gnash-commit] gnash ChangeLog server/parser/action_buffer.cpp...,
Benjamin Wolsey <=