Hello! This patch is based on some work[1] I did to make the poppler Object union member accesses safer in the future (to avoid things like CVE-2008-1693). After some discussion on IRC, an improved way of handling things was resolved, which should not degrade performance in any noticeable way as the checks are memory-local to the union member being accessed, and can be hinted to be "unlikely" situations by the compiler.
Please keep me in CC, as I'm on the list. Thanks! -Kees [1] http://bugs.freedesktop.org/show_bug.cgi?id=11392 --- Provide stronger type-check during Object union access to avoid mis-using pointers or uninitialized memory. Signed-off-by: Kees Cook <[EMAIL PROTECTED]> --- Object.h | 68 +++++++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/poppler/Object.h b/poppler/Object.h index 4d64795..19cec9d 100644 --- a/poppler/Object.h +++ b/poppler/Object.h @@ -18,6 +18,22 @@ #include "goo/gtypes.h" #include "goo/gmem.h" #include "goo/GooString.h" +#include "Error.h" + +#if defined(__GNUC__) && (__GNUC__ > 2) && defined(__OPTIMIZE__) +# define likely(x) __builtin_expect((x), 1) +# define unlikely(x) __builtin_expect((x), 0) +#else +# define likely(x) (x) +# define unlikely(x) (x) +#endif + +#define OBJECT_TYPE_CHECK(wanted_type) \ + if (unlikely(type != wanted_type)) { \ + error(0, "Call to Object where the object was type %d, " \ + "not the expected type %d", type, wanted_type); \ + abort(); \ + } class XRef; class Array; @@ -66,17 +82,19 @@ enum ObjType { //------------------------------------------------------------------------ #ifdef DEBUG_MEM -#define initObj(t) ++numAlloc[type = t] +#define initObj(t) zeroUnion(); ++numAlloc[type = t] #else -#define initObj(t) type = t +#define initObj(t) zeroUnion(); type = t #endif class Object { public: + // clear the anonymous union as best we can -- clear at least a pointer + void zeroUnion() { this->name = NULL; } // Default constructor. Object(): - type(objNone) {} + type(objNone) { zeroUnion(); } // Initialize an object. Object *initBool(GBool boolnA) @@ -223,16 +241,16 @@ private: #include "Array.h" inline int Object::arrayGetLength() - { return array->getLength(); } + { OBJECT_TYPE_CHECK(objArray); return array->getLength(); } inline void Object::arrayAdd(Object *elem) - { array->add(elem); } + { OBJECT_TYPE_CHECK(objArray); array->add(elem); } inline Object *Object::arrayGet(int i, Object *obj) - { return array->get(i, obj); } + { OBJECT_TYPE_CHECK(objArray); return array->get(i, obj); } inline Object *Object::arrayGetNF(int i, Object *obj) - { return array->getNF(i, obj); } + { OBJECT_TYPE_CHECK(objArray); return array->getNF(i, obj); } //------------------------------------------------------------------------ // Dict accessors. @@ -241,34 +259,34 @@ inline Object *Object::arrayGetNF(int i, Object *obj) #include "Dict.h" inline int Object::dictGetLength() - { return dict->getLength(); } + { OBJECT_TYPE_CHECK(objDict); return dict->getLength(); } inline void Object::dictAdd(char *key, Object *val) - { dict->add(key, val); } + { OBJECT_TYPE_CHECK(objDict); dict->add(key, val); } inline void Object::dictSet(char *key, Object *val) - { dict->set(key, val); } + { OBJECT_TYPE_CHECK(objDict); dict->set(key, val); } inline GBool Object::dictIs(char *dictType) - { return dict->is(dictType); } + { OBJECT_TYPE_CHECK(objDict); return dict->is(dictType); } inline GBool Object::isDict(char *dictType) { return type == objDict && dictIs(dictType); } inline Object *Object::dictLookup(char *key, Object *obj) - { return dict->lookup(key, obj); } + { OBJECT_TYPE_CHECK(objDict); return dict->lookup(key, obj); } inline Object *Object::dictLookupNF(char *key, Object *obj) - { return dict->lookupNF(key, obj); } + { OBJECT_TYPE_CHECK(objDict); return dict->lookupNF(key, obj); } inline char *Object::dictGetKey(int i) - { return dict->getKey(i); } + { OBJECT_TYPE_CHECK(objDict); return dict->getKey(i); } inline Object *Object::dictGetVal(int i, Object *obj) - { return dict->getVal(i, obj); } + { OBJECT_TYPE_CHECK(objDict); return dict->getVal(i, obj); } inline Object *Object::dictGetValNF(int i, Object *obj) - { return dict->getValNF(i, obj); } + { OBJECT_TYPE_CHECK(objDict); return dict->getValNF(i, obj); } //------------------------------------------------------------------------ // Stream accessors. @@ -277,33 +295,33 @@ inline Object *Object::dictGetValNF(int i, Object *obj) #include "Stream.h" inline GBool Object::streamIs(char *dictType) - { return stream->getDict()->is(dictType); } + { OBJECT_TYPE_CHECK(objStream); stream->getDict()->is(dictType); } inline GBool Object::isStream(char *dictType) { return type == objStream && streamIs(dictType); } inline void Object::streamReset() - { stream->reset(); } + { OBJECT_TYPE_CHECK(objStream); stream->reset(); } inline void Object::streamClose() - { stream->close(); } + { OBJECT_TYPE_CHECK(objStream); stream->close(); } inline int Object::streamGetChar() - { return stream->getChar(); } + { OBJECT_TYPE_CHECK(objStream); return stream->getChar(); } inline int Object::streamLookChar() - { return stream->lookChar(); } + { OBJECT_TYPE_CHECK(objStream); return stream->lookChar(); } inline char *Object::streamGetLine(char *buf, int size) - { return stream->getLine(buf, size); } + { OBJECT_TYPE_CHECK(objStream); return stream->getLine(buf, size); } inline Guint Object::streamGetPos() - { return stream->getPos(); } + { OBJECT_TYPE_CHECK(objStream); return stream->getPos(); } inline void Object::streamSetPos(Guint pos, int dir) - { stream->setPos(pos, dir); } + { OBJECT_TYPE_CHECK(objStream); stream->setPos(pos, dir); } inline Dict *Object::streamGetDict() - { return stream->getDict(); } + { OBJECT_TYPE_CHECK(objStream); return stream->getDict(); } #endif -- Kees Cook @outflux.net _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
