[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Gnash-commit] gnash ChangeLog libamf/amf.cpp libamf/amf.h lib...
|
From: |
Sandro Santilli |
|
Subject: |
[Gnash-commit] gnash ChangeLog libamf/amf.cpp libamf/amf.h lib... |
|
Date: |
Wed, 30 Apr 2008 18:19:41 +0000 |
CVSROOT: /sources/gnash
Module name: gnash
Changes by: Sandro Santilli <strk> 08/04/30 18:19:41
Modified files:
. : ChangeLog
libamf : amf.cpp amf.h lcshm.cpp lcshm.h sol.cpp
libnet : rtmp.cpp
Log message:
Add boundary checking in AMF parser, will throw
a ParserException on premature end of buffer,
sol::readFile will interchept and log_error.
CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.6468&r2=1.6469
http://cvs.savannah.gnu.org/viewcvs/gnash/libamf/amf.cpp?cvsroot=gnash&r1=1.75&r2=1.76
http://cvs.savannah.gnu.org/viewcvs/gnash/libamf/amf.h?cvsroot=gnash&r1=1.41&r2=1.42
http://cvs.savannah.gnu.org/viewcvs/gnash/libamf/lcshm.cpp?cvsroot=gnash&r1=1.14&r2=1.15
http://cvs.savannah.gnu.org/viewcvs/gnash/libamf/lcshm.h?cvsroot=gnash&r1=1.10&r2=1.11
http://cvs.savannah.gnu.org/viewcvs/gnash/libamf/sol.cpp?cvsroot=gnash&r1=1.34&r2=1.35
http://cvs.savannah.gnu.org/viewcvs/gnash/libnet/rtmp.cpp?cvsroot=gnash&r1=1.8&r2=1.9
Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.6468
retrieving revision 1.6469
diff -u -b -r1.6468 -r1.6469
--- ChangeLog 30 Apr 2008 18:02:05 -0000 1.6468
+++ ChangeLog 30 Apr 2008 18:19:38 -0000 1.6469
@@ -1,3 +1,11 @@
+2008-04-30 Sandro Santilli <address@hidden>
+
+ * libamf/amf.{cpp,h}, libamf/lcshm.{cpp,h},
+ libamf/sol.cpp, libnet/rtmp.cpp:
+ Add boundary checking in AMF parser, will throw
+ a ParserException on premature end of buffer,
+ sol::readFile will interchept and log_error.
+
2008-04-30 Rob Savoye <address@hidden>
* configure.ac: Always look for mallinfo(), jemalloc now supports it.
Index: libamf/amf.cpp
===================================================================
RCS file: /sources/gnash/gnash/libamf/amf.cpp,v
retrieving revision 1.75
retrieving revision 1.76
diff -u -b -r1.75 -r1.76
--- libamf/amf.cpp 30 Apr 2008 13:15:33 -0000 1.75
+++ libamf/amf.cpp 30 Apr 2008 18:19:40 -0000 1.76
@@ -21,6 +21,8 @@
#include "gnashconfig.h"
#endif
+#include "GnashException.h"
+
#include <string>
#include <vector>
@@ -44,6 +46,12 @@
namespace amf
{
+#define ENSUREBYTES(from, toofar, size) { \
+ if ( from+size >= toofar ) \
+ throw ParserException("Premature end of AMF stream"); \
+}
+
+
// These are used to print more intelligent debug messages
const char *amftype_str[] = {
"Number",
@@ -726,11 +734,14 @@
AMF::extractAMF(Buffer *buf)
{
// GNASH_REPORT_FUNCTION;
- return extractAMF(buf->reference());
+ Network::byte_t* start = buf->reference();
+ Network::byte_t* tooFar = start+buf->size();
+
+ return extractAMF(start, tooFar);
}
Element *
-AMF::extractAMF(Network::byte_t *in)
+AMF::extractAMF(Network::byte_t *in, Network::byte_t* tooFar)
{
// GNASH_REPORT_FUNCTION;
@@ -757,6 +768,9 @@
// mostly to make valgrind shut up, as it has a tendency to
// complain about legit code when it comes to all this byte
// manipulation stuff.
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, 1);
+#endif
char c = *(reinterpret_cast<char *>(tmpptr));
Element::amf0_type_e type = static_cast<Element::amf0_type_e>(c);
tmpptr++; // skip the header byte
@@ -764,20 +778,32 @@
AMF amf_obj;
switch (type) {
case Element::NUMBER_AMF0:
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, AMF0_NUMBER_SIZE);
+#endif
el->makeNumber(tmpptr);
tmpptr += AMF0_NUMBER_SIZE; // all numbers are 8 bit big endian
break;
case Element::BOOLEAN_AMF0:
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, sizeof(boost::uint16_t));
+#endif
el->makeBoolean(tmpptr);
tmpptr += sizeof(boost::uint16_t); // although a bool is one byte,
it's stored as a short
break;
case Element::STRING_AMF0:
// get the length of the name
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, sizeof(boost::uint16_t));
+#endif
length = ntohs((*(boost::uint16_t *)tmpptr) & 0xffff);
tmpptr += sizeof(boost::uint16_t);
log_debug(_("AMF String length is: %d"), length);
if (length > 0) {
// get the name of the element
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, length);
+#endif
el->makeString(tmpptr, length);
log_debug(_("AMF String is: %s"), el->to_string());
tmpptr += length;
@@ -789,10 +815,13 @@
el->makeObject();
Element *child;
do {
- child = amf_obj.extractProperty(tmpptr);
+ child = amf_obj.extractProperty(tmpptr, tooFar);
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, amf_obj.totalsize()-1);
+#endif
tmpptr += amf_obj.totalsize() - 1;
el->addProperty(child);
- } while (*tmpptr != Element::OBJECT_END_AMF0);
+ } while (tmpptr < tooFar && *tmpptr != Element::OBJECT_END_AMF0);
break;
case Element::MOVIECLIP_AMF0:
case Element::NULL_AMF0:
@@ -822,17 +851,24 @@
AMF::extractProperty(Buffer *buf)
{
// GNASH_REPORT_FUNCTION;
- return extractProperty(buf->reference());
+
+ Network::byte_t* start = buf->reference();
+ Network::byte_t* tooFar = start+buf->size();
+ return extractProperty(start, tooFar);
}
Element *
-AMF::extractProperty(Network::byte_t *in)
+AMF::extractProperty(Network::byte_t *in, Network::byte_t* tooFar)
{
// GNASH_REPORT_FUNCTION;
Network::byte_t *tmpptr = in;
boost::uint16_t length = 0;
Network::byte_t len = 0;
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, sizeof(boost::uint16_t));
+#endif
+
length = *(reinterpret_cast<boost::uint16_t *>(tmpptr));
// length = tmpptr[1]; // FIXME: hack. This supports
// // strings up to 256 characters
@@ -854,15 +890,22 @@
// get the name of the element, the length of which we just decoded
if (length > 0) {
// log_debug(_("AMF element length is: %d"), length);
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, length);
+#endif
el->setName(tmpptr, length);
// log_debug(_("AMF element name is: %s"), el->getName());
tmpptr += length;
}
// get the type of the element, which is a single byte.
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, 1);
+#endif
char c = *(reinterpret_cast<char *>(tmpptr));
Element::amf0_type_e type = static_cast<Element::amf0_type_e>(c);
tmpptr++;
+
if (type != Element::TYPED_OBJECT_AMF0) {
// log_debug(_("AMF type is: %s"), amftype_str[(int)type]);
el->setType(type);
@@ -871,6 +914,9 @@
switch (type) {
case Element::NUMBER_AMF0:
{
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, sizeof(double));
+#endif
double num = *reinterpret_cast<const double*>(tmpptr);
swapBytes(&num, AMF0_NUMBER_SIZE);
el->makeNumber(num);
@@ -879,6 +925,12 @@
}
case Element::BOOLEAN_AMF0:
{
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, 1);
+#endif
+ //
+ // @@strk@@ : we read 2 bytes from ::extractAMF, why only 1 here in
::extractProperty ?
+ //
bool sheet = *(reinterpret_cast<bool *>(tmpptr));
el->makeBoolean(sheet);
tmpptr += 1;
@@ -887,10 +939,16 @@
case Element::STRING_AMF0:
// extractString returns a printable char *. First 2 bytes for the
length,
// and then read the string, which is NOT NULL terminated.
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, sizeof(boost::uint16_t));
+#endif
length = *reinterpret_cast<boost::uint16_t *>(tmpptr);
swapBytes(&length, sizeof(boost::uint16_t));
tmpptr += sizeof(boost::uint16_t);
if (length > 0) {
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(tmpptr, tooFar, length);
+#endif
el->makeString(tmpptr, length);
tmpptr += length;
}
@@ -901,7 +959,7 @@
// log_debug(_("Property \"%s\" is: %s"), el->getName(),
el->to_string());
break;
case Element::OBJECT_AMF0:
- while (*(tmpptr++) != Element::OBJECT_END_AMF0) {
+ while ( (tmpptr<tooFar) && ( *(tmpptr++) != Element::OBJECT_END_AMF0)
) {
// log_debug("Looking for end of object...");
}
break;
@@ -920,11 +978,11 @@
el->makeObjectEnd();
break;
case Element::TYPED_OBJECT_AMF0:
- el->makeTypedObject(tmpptr, 0);
+ el->makeTypedObject(tmpptr, 0); // TODO: pass tooFar over ?
break;
case Element::STRICT_ARRAY_AMF0:
case Element::DATE_AMF0:
- el->makeDate(tmpptr);
+ el->makeDate(tmpptr); // TODO: pass tooFar over ?
break;
case Element::LONG_STRING_AMF0:
case Element::UNSUPPORTED_AMF0:
Index: libamf/amf.h
===================================================================
RCS file: /sources/gnash/gnash/libamf/amf.h,v
retrieving revision 1.41
retrieving revision 1.42
diff -u -b -r1.41 -r1.42
--- libamf/amf.h 30 Apr 2008 03:35:30 -0000 1.41
+++ libamf/amf.h 30 Apr 2008 18:19:40 -0000 1.42
@@ -246,13 +246,40 @@
// to keep track where we are in the memory buffer so these can't
// be static.
- // Extract an AMF object. These have no name like the variables do.
- amf::Element *extractAMF(gnash::Network::byte_t *in);
+ /// Extract an AMF object. These have no name like the variables do.
+ //
+ /// @param in
+ /// Pointer to start parsing from
+ //
+ /// @param tooFar
+ /// A pointer to one-byte-past the last valid memory
+ /// address within the buffer.
+ ///
+ /// May throw a ParserException
+ ///
+ amf::Element *extractAMF(gnash::Network::byte_t *in,
gnash::Network::byte_t* tooFar);
+
+ /// Extract an AMF object. These have no name like the variables do.
amf::Element *extractAMF(Buffer *buf);
- // Extract an AMF "variable", which is a standard AMF object preceeded by
- // just a length and a name field.
- amf::Element *extractProperty(gnash::Network::byte_t *in);
+ /// \brief
+ /// Extract an AMF "variable", which is a standard AMF object preceeded by
+ /// just a length and a name field.
+ ///
+ /// @param in
+ /// Pointer to start parsing property from
+ //
+ /// @param tooFar
+ /// A pointer to one-byte-past the last valid memory
+ /// address within the buffer.
+ ///
+ /// May throw a ParserException
+ ///
+ amf::Element *extractProperty(gnash::Network::byte_t *in,
gnash::Network::byte_t* tooFar);
+
+ /// \brief
+ /// Extract an AMF "variable", which is a standard AMF object preceeded by
+ /// just a length and a name field.
amf::Element *extractProperty(Buffer *buf);
size_t totalsize() { return _totalsize; }
Index: libamf/lcshm.cpp
===================================================================
RCS file: /sources/gnash/gnash/libamf/lcshm.cpp,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -b -r1.14 -r1.15
--- libamf/lcshm.cpp 30 Apr 2008 03:35:31 -0000 1.14
+++ libamf/lcshm.cpp 30 Apr 2008 18:19:40 -0000 1.15
@@ -33,6 +33,7 @@
#include "amf.h"
#include "shm.h"
#include "element.h"
+#include "GnashException.h"
#include "lcshm.h"
using namespace std;
@@ -59,6 +60,11 @@
# define MAXHOSTNAMELEN 64
#endif
+#define ENSUREBYTES(from, toofar, size) { \
+ if ( from+size >= toofar ) \
+ throw ParserException("Premature end of AMF stream"); \
+}
+
// \class LocalConnection
/// \brief Open a connection between two SWF movies so they can send
/// each other Flash Objects to be executed.
@@ -294,7 +300,7 @@
// other web sites have claimed there is a length field in the initial
// shared memory segment header, I've never seen one in my tests.
Network::byte_t *
-LcShm::parseHeader(Network::byte_t *data)
+LcShm::parseHeader(Network::byte_t *data, Network::byte_t* tooFar)
{
// GNASH_REPORT_FUNCTION;
Network::byte_t *ptr = data;
@@ -304,6 +310,10 @@
return 0;
}
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(ptr, tooFar, LC_HEADER_SIZE);
+#endif
+
memcpy(&_header, ptr, LC_HEADER_SIZE);
// memcpy(&_object, data + LC_HEADER_SIZE, _header.length);
// log_debug("Timestamp: %d", _header.timestamp);
@@ -314,7 +324,7 @@
AMF amf;
- Element *el = amf.extractAMF(ptr);
+ Element *el = amf.extractAMF(ptr, tooFar);
if (el == 0) {
log_debug("Didn't extract an element from the byte stream!");
return 0;
@@ -323,7 +333,7 @@
_object.connection_name = el->to_string();
delete el;
- el = amf.extractAMF(ptr);
+ el = amf.extractAMF(ptr, tooFar);
if (ptr != 0) {
_object.hostname = el->to_string();
}
@@ -412,9 +422,11 @@
return false;
}
- Listener::setBaseAddress(reinterpret_cast<uint8_t *>(Shm::getAddr()));
- _baseaddr = reinterpret_cast<uint8_t *>(Shm::getAddr());
- parseHeader(Listener::getBaseAddress());
+ uint8_t* baseAddress = reinterpret_cast<uint8_t *>(Shm::getAddr());
+ uint8_t* tooFar = baseAddress+Shm::getSize();
+ Listener::setBaseAddress(baseAddress);
+ _baseaddr = baseAddress;
+ parseHeader(baseAddress, tooFar);
// vector<amf::Element *> ellist = parseBody(ptr);
// log_debug("Base address is: 0x%x, 0x%x",
// (unsigned int)Listener::getBaseAddress(), (unsigned
int)_baseaddr);
@@ -438,9 +450,11 @@
return false;
}
- Listener::setBaseAddress(reinterpret_cast<uint8_t *>(Shm::getAddr()));
- _baseaddr = reinterpret_cast<uint8_t *>(Shm::getAddr());
- parseHeader(Listener::getBaseAddress());
+ uint8_t* baseAddress = reinterpret_cast<uint8_t *>(Shm::getAddr());
+ uint8_t* tooFar = baseAddress+Shm::getSize();
+ Listener::setBaseAddress(baseAddress);
+ _baseaddr = baseAddress;
+ parseHeader(baseAddress, tooFar);
// vector<amf::Element *> ellist = parseBody(ptr);
// log_debug("Base address is: 0x%x, 0x%x",
// (unsigned int)Listener::getBaseAddress(), (unsigned
int)_baseaddr);
Index: libamf/lcshm.h
===================================================================
RCS file: /sources/gnash/gnash/libamf/lcshm.h,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -b -r1.10 -r1.11
--- libamf/lcshm.h 30 Mar 2008 22:08:54 -0000 1.10
+++ libamf/lcshm.h 30 Apr 2008 18:19:40 -0000 1.11
@@ -86,7 +86,18 @@
std::vector<amf::Element *> &data);
void recv(std::string &name, std::string &dataname, amf::Element *data);
std::vector<amf::Element *> parseBody(gnash::Network::byte_t *data);
- gnash::Network::byte_t *parseHeader(gnash::Network::byte_t *data);
+
+ /// @param in
+ /// Pointer to start parsing from
+ //
+ /// @param tooFar
+ /// A pointer to one-byte-past the last valid memory
+ /// address within the buffer.
+ ///
+ /// May throw a ParserException
+ ///
+ gnash::Network::byte_t *parseHeader(gnash::Network::byte_t *data,
gnash::Network::byte_t* tooFar);
+
gnash::Network::byte_t *formatHeader(const std::string &con, const
std::string &host, bool domain);
void addConnectionName(std::string &name);
void addHostname(std::string &name);
Index: libamf/sol.cpp
===================================================================
RCS file: /sources/gnash/gnash/libamf/sol.cpp,v
retrieving revision 1.34
retrieving revision 1.35
diff -u -b -r1.34 -r1.35
--- libamf/sol.cpp 30 Apr 2008 03:35:31 -0000 1.34
+++ libamf/sol.cpp 30 Apr 2008 18:19:41 -0000 1.35
@@ -34,6 +34,7 @@
#include "buffer.h"
#include "sol.h"
#include "log.h"
+#include "GnashException.h"
#if defined(HAVE_WINSOCK_H) && !defined(__OS2__)
# include <winsock2.h>
@@ -67,6 +68,11 @@
//char *SOL_FILETYPE = "TCSO";
const short SOL_BLOCK_MARK = 0x0004;
+#define ENSUREBYTES(from, toofar, size) { \
+ if ( from+size >= toofar ) \
+ throw ParserException("Premature end of AMF stream"); \
+}
+
namespace amf
{
@@ -326,14 +332,22 @@
// Make sure it's an SOL file
if (stat(filespec.c_str(), &st) == 0) {
+
+try {
ifstream ifs(filespec.c_str(), ios::binary);
_filesize = st.st_size;
buf = new Network::byte_t[_filesize + sizeof(int)];
ptr = buf;
+ Network::byte_t* tooFar = buf+_filesize+sizeof(int);
+
bodysize = st.st_size - 6;
_filespec = filespec;
ifs.read(reinterpret_cast<char *>(ptr), _filesize);
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(ptr, tooFar, 2+4+10); // magic number, file size, file
marker
+#endif
+
// skip the magic number (will check later)
ptr += 2;
@@ -358,22 +372,33 @@
log_error("%s isn't an SOL file", filespec.c_str());
}
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(ptr, tooFar, 2);
+#endif
+
// 2 bytes for the length of the object name, but it's also null
terminated
size = *(reinterpret_cast<boost::uint16_t *>(ptr));
size = ntohs(size);
ptr += 2;
- _objname = reinterpret_cast<const char *>(ptr);
+#ifndef GNASH_TRUST_AMF
+ ENSUREBYTES(ptr, tooFar, size+4); // 4 is the padding below
+#endif
+
+ // TODO: make sure there's the null-termination, or
+ // force if if it's there by definition
+ _objname = reinterpret_cast<const char *>(ptr);
ptr += size;
+
// Go past the padding
ptr += 4;
AMF amf_obj;
int size = 0;
amf::Element *el;
- while (size < static_cast<boost::uint16_t>(bodysize) - 24) {
+ while ( size < static_cast<boost::uint16_t>(bodysize) - 24 ) {
if (ptr) {
- el = amf_obj.extractProperty(ptr);
+ el = amf_obj.extractProperty(ptr, tooFar);
if (el != 0) {
size += amf_obj.totalsize();
ptr += amf_obj.totalsize();
@@ -390,6 +415,11 @@
ifs.close();
return true;
+} catch (std::exception& e) {
+ log_error("Reading SharedObject %s: %s", filespec, e.what());
+ return false;
+}
+
}
// log_error("Couldn't open file: %s", strerror(errno));
Index: libnet/rtmp.cpp
===================================================================
RCS file: /sources/gnash/gnash/libnet/rtmp.cpp,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -b -r1.8 -r1.9
--- libnet/rtmp.cpp 30 Apr 2008 03:48:52 -0000 1.8
+++ libnet/rtmp.cpp 30 Apr 2008 18:19:41 -0000 1.9
@@ -298,6 +298,7 @@
int packetsize = 0;
unsigned int amf_index, headersize;
Network::byte_t *ptr = buf->reference();
+ Network::byte_t *tooFar = ptr+buf->size();
AMF amf;
//
address@hidden@\000\000\000\000\000\000\003\000\003app\002\000#software/gnash/tests/1153948634.flv\000\bflashVer\002\000\fLNX
6,0,82,0\000\006swfUrl\002\000\035file:///file|address@hidden://localhost/software/gnash/tests/1153948634
@@ -326,14 +327,14 @@
ptr = parseHeader(ptr);
// ptr += headersize;
- amf::Element *el = amf.extractAMF(ptr);
+ amf::Element *el = amf.extractAMF(ptr, tooFar);
el->dump();
- el = amf.extractAMF(ptr) + 1;
+ el = amf.extractAMF(ptr, tooFar) + 1; // @@strk@@ : what's the +1 for ?
el->dump();
log_debug (_("Reading AMF packets till we're done..."));
buf->dump();
while (ptr < end) {
- amf::Element *el = amf.extractProperty(ptr);
+ amf::Element *el = amf.extractProperty(ptr, tooFar);
addProperty(el);
el->dump();
}
@@ -346,7 +347,7 @@
buf = _handler->merge(buf);
}
while ((ptr - buf->begin()) < actual_size) {
- amf::Element *el = amf.extractProperty(ptr);
+ amf::Element *el = amf.extractProperty(ptr, tooFar);
addProperty(el);
el->dump(); // FIXME: dump the AMF objects as they are read
in
}