Re: [RFC] handling of malformed documents

[Michal H. <ms...@gm...>]

Just for reference. This patch solves bug
http://pdfedit.petricek.net/bt/view.php?id=199

On Tue, Mar 04, 2008 at 09:23:10PM +0100, Michal Hocko wrote:
> Hi.
> Attached patch adds support for correct handling of malformed documents.
> It changes xpdf code as well as kernel code, so it needs good and deep
> inspection. I have run some tests and it is seems to be working.
> 
> Background information:
> xpdf
> ====
> Parser::getObj semantics has changed slightly. Normally getObj returns
> pointer to the given object which is filled with parsed stream content.
> It tried to overcome inconsistencies simply by ignoring them. Error
> could be detected only by reaching end of data in the stream. This way,
> however, could lead to unpredictable states and crashes. 
> This was changed to report each error. This is done by returning NULL.
> It is rather intrusive change, because all the code assumes that getObj
> never returns NULL. Patch should handle all Parser::getObj (don't change
> with Lexer::getObj which was not touched) except for tests (this should
> be done too of course but it has lower priority).
> Also Gfx::go method changed to return value (is was void before). It
> uses Parser internally so had to be. If there is a problem, it returns -1.
> Both methods are marked with warn_unused_result gcc attribute which
> displays warning if the return value is not used or checked.
> XRef::{ok,erroCode} internal indicator of problem is now handled by
> setErrCode method, which handles both of them with the tight logic. This
> means that all code should allways ask XRef::isOk and only if it is
> false, then to check XRef::getErrCode (this was done). 
> 
> kernel
> ======
> CXref::fetch method now may throw MalformedFormatException exception if
> parsing failed on the bottom of hierarchy. 
> Also all methods which use parser directly should throw
> MalformedFormatException exception too (this was done, but I am not very
> sure at some places).
> 
> 
> Feel free to ask, if something is not clear.
> -- 
> Michal Hocko

> Index: src/kernel/cobject2xpdf.cc
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/kernel/cobject2xpdf.cc,v
> retrieving revision 1.5
> diff -u -p -r1.5 cobject2xpdf.cc
> --- src/kernel/cobject2xpdf.cc	25 Feb 2008 19:54:15 -0000	1.5
> +++ src/kernel/cobject2xpdf.cc	4 Mar 2008 20:12:45 -0000
> @@ -826,7 +826,11 @@ xpdfObjFromString (const std::string& st
>  	// Get xpdf obj from the stream
>  	//
>  	::Object* obj = XPdfObjectFactory::getInstance();
> -	parser->getObj (obj);
> +	if(!parser->getObj (obj)) {
> +		utilsPrintDbg(debug::DBG_ERR, "Unable to parse string \""<< 
> +				str <<"\"");
> +		throw MalformedFormatExeption("bad string");
> +	}
>  
>  	//
>  	// If xpdf returned objNull and we did not give him null, an error occured
> Index: src/kernel/cstream.cc
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/kernel/cstream.cc,v
> retrieving revision 1.17
> diff -u -p -r1.17 cstream.cc
> --- src/kernel/cstream.cc	22 Feb 2008 08:42:08 -0000	1.17
> +++ src/kernel/cstream.cc	4 Mar 2008 20:12:45 -0000
> @@ -26,6 +26,7 @@
>  #include "kernel/static.h"
>  #include "kernel/cstream.h"
>  #include "kernel/cpdf.h"
> +#include "kernel/exceptions.h"
>  
>  //=====================================================================================
>  namespace pdfobjects {
> @@ -468,7 +469,10 @@ CStream::getXpdfObject (::Object& obj)
>  	{
>  		curObj.free ();
>  		
> -		parser->getObj (&curObj);
> +		if(!parser->getObj (&curObj)) {	
> +			kernelPrintDbg(debug::DBG_ERR, "Unable to parse content stream");
> +			throw MalformedFormatExeption("bad stream");
> +		}
>  		assert (!curObj.isNone ());
>  		assert (!curObj.isNull ());
>  		assert (!curObj.isError ());
> Index: src/kernel/cstreamsxpdfreader.h
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/kernel/cstreamsxpdfreader.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 cstreamsxpdfreader.h
> --- src/kernel/cstreamsxpdfreader.h	22 Feb 2008 08:42:08 -0000	1.23
> +++ src/kernel/cstreamsxpdfreader.h	4 Mar 2008 20:12:45 -0000
> @@ -148,11 +148,17 @@ public:
>  		close();
>  	}
>  
> -	/** Get xpdf object. */
> +	/** Get xpdf object.
> +	 * @throw MalformedFormatExeption if not able to parse object.
> +	 */
>  	void getXpdfObject (::Object& obj)
>  	{
>  		curobj.reset ();
> -		parser->getObj (curobj.get());
> +		if(!parser->getObj (curobj.get()))
> +		{
> +			kernelPrintDbg(debug::DBG_ERR, "Unabel to parse object");
> +			throw MalformedFormatExeption("content stream parse");
> +		}
>  		curobj->copy (&obj);
>  	}
>  
> Index: src/kernel/stateupdater.cc
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/kernel/stateupdater.cc,v
> retrieving revision 1.21
> diff -u -p -r1.21 stateupdater.cc
> --- src/kernel/stateupdater.cc	22 Feb 2008 08:42:08 -0000	1.21
> +++ src/kernel/stateupdater.cc	4 Mar 2008 20:12:45 -0000
> @@ -806,7 +806,8 @@ StateUpdater::printTextUpdate (GfxState*
>  			  while (!parser->eofOfActualStream() && !obj->isCmd("BI"))
>  			  {
>  				obj.reset ();
> -				parser->getObj (obj.get());
> +				if(!parser->getObj (obj.get())) 
> +					goto malformedErr;
>  			  }
>  
>  			  if (!parser->eofOfActualStream())
> @@ -814,7 +815,8 @@ StateUpdater::printTextUpdate (GfxState*
>  				  //
>  				  // Find height information in image stream dictionary
>  				  //
> -				  parser->getObj (obj.get());
> +				  if(!parser->getObj (obj.get())) 
> +					  goto malformedErr;
>  				  while (!obj->isCmd("ID") && !obj->isEOF()) 
>  				  { 
>  					if (obj->isName()) 
> @@ -823,7 +825,8 @@ StateUpdater::printTextUpdate (GfxState*
>  					  string key (copyString (obj->getName()));
>  					  obj.reset();
>  					  // Get key
> -					  parser->getObj(obj.get());
> +					  if(!parser->getObj(obj.get())) 
> +						  goto malformedErr;
>  					  if (obj->isEOF() || obj->isError()) 
>  						break;
>  					  //
> @@ -846,7 +849,8 @@ StateUpdater::printTextUpdate (GfxState*
>  						kernelPrintDbg (DBG_DBG, "Bad inline image dictionary.");
>  						break;
>  					}
> -					parser->getObj(obj.get());
> +					if(!parser->getObj(obj.get())) 
> +						goto malformedErr;
>  				  }
>  
>  			   } //  if (parser->eofOfActualStream())
> @@ -925,6 +929,10 @@ StateUpdater::printTextUpdate (GfxState*
>  
>  	// return changed state
>  	return state;
> +
> +malformedErr:
> +	kernelPrintDbg(DBG_ERR, "Unable to parse content stream");
> +	throw MalformedFormatExeption("content stream error");
>  }	
>  
>  
> Index: src/kernel/xrefwriter.cc
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/kernel/xrefwriter.cc,v
> retrieving revision 1.42
> diff -u -p -r1.42 xrefwriter.cc
> --- src/kernel/xrefwriter.cc	4 Mar 2008 16:25:00 -0000	1.42
> +++ src/kernel/xrefwriter.cc	4 Mar 2008 20:12:45 -0000
> @@ -52,24 +52,36 @@ bool checkLinearized(StreamWriter & stre
>  	Object obj;
>  	Stream * subStr = stream.makeSubStream(stream.getPos(), false, 0, &obj);
>  	Parser parser=Parser(xref, new Lexer(NULL, subStr), gTrue);
> +	
> +	//  result is false by default - it MUST BE linearized dictionary
> +	bool result = false;
>  
> -	while(subStr->getPos() < FIRST_LINEARIZED_BLOCK)
> +	while(!result && subStr->getPos() < FIRST_LINEARIZED_BLOCK)
>  	{
>  		Object obj1, obj2, obj3;
> -		parser.getObj(&obj1);
> -		parser.getObj(&obj2);
> -		parser.getObj(&obj3);
> +		if(!parser.getObj(&obj1))
> +			goto malformedErr;
> +		if(!parser.getObj(&obj2))
> +		{
> +			obj1.free();
> +			goto malformedErr;
> +		}
> +		if(!parser.getObj(&obj3))
> +		{
> +			obj1.free();
> +			obj2.free();
> +			goto malformedErr;
> +		}
>  
>  		// indirect object must start with 
>  		// num gen obj line
>  		if(obj1.isInt() && obj2.isInt() && obj3.isCmd())
>  		{
>  			Object obj;
> -			parser.getObj(&obj);
> +			if(!parser.getObj(&obj))
> +				goto malformedErr;
> + 	
>  
> -			//  result is false by default - it MUST BE linearized dictionary
> -			bool result=false;
> -			
>  			if(obj.isDict())
>  			{
>  				// indirect object is dictionary, so it can be Linearized
> @@ -90,11 +102,6 @@ bool checkLinearized(StreamWriter & stre
>  				}
>  			}
>  			obj.free();
> -
> -			obj1.free();
> -			obj2.free();
> -			obj3.free();
> -			return result;
>  		}
>  
>  		obj1.free();
> @@ -103,7 +110,11 @@ bool checkLinearized(StreamWriter & stre
>  	}
>  
>  	// no indirect object in the document
> -	return false;
> +	return result;
> +
> +malformedErr:
> +	utilsPrintDbg(DBG_ERR, "Unable to parse stream - bad content.");
> +	throw MalformedFormatExeption("bad stream");
>  }
>  
>  } // end of utils namespace
> @@ -456,10 +467,16 @@ int XRefWriter::getOldStyleTrailer(Objec
>  
>  			// deallocates previous one before it is filled by new one
>  			trailer->free();
> -			parser.getObj(trailer);
> +			if(!parser.getObj(trailer))
> +			{
> +				kernelPrintDbg(DBG_ERR, "Unable to parse trailer");
> +				return -1;
> +			}
>  			if(!trailer->isDict())
>  			{
> -				kernelPrintDbg(DBG_ERR, "Trailer is not dictionary.");
> +				kernelPrintDbg(DBG_ERR, "Trailer is not dictionary. (type="
> +						<<trailer->getType() << ")");
> +				trailer->free();
>  				return -1;
>  			}
>  
> @@ -478,20 +495,23 @@ int XRefWriter::getStreamTrailer(Object 
>  {
>  	// gen number should follow
>  	::Object obj;
> -	parser.getObj(&obj);
> +	if(!parser.getObj(&obj))
> +		goto malformedErr;
>  	if(!obj.isInt())
>  		goto bad_header;
>  	// We don't need to free obj here, because int is not
>  	// allocated internally
>  	// end of indirect object header is obj key word
> -	parser.getObj(&obj);
> +	if(!parser.getObj(&obj))
> +		goto malformedErr;
>  	if(!obj.isCmd("obj"))
>  		goto bad_header;
>  
>  	// header of indirect object is ok, so parse trailer object
>  	obj.free();
>  	trailer->free();
> -	parser.getObj(trailer);
> +	if(!parser.getObj(trailer))
> +		goto malformedErr;
>  	if(!trailer->isStream())
>  	{
>  		kernelPrintDbg(DBG_ERR, "Trailer is supposed to be stream, but "
> @@ -503,7 +523,9 @@ int XRefWriter::getStreamTrailer(Object 
>  	// stream object and uses just dictionary part
>  	kernelPrintDbg(DBG_INFO, "New xref stream section. off="<<off);
>  	return 0;
> -
> +malformedErr:
> +	kernelPrintDbg(DBG_ERR, "Unable to parse trailer");
> +	return -1;
>  bad_header:
>  	kernelPrintDbg(DBG_ERR, "Trailer header corrupted.");
>  	obj.free();
> @@ -576,7 +598,11 @@ void XRefWriter::collectRevisions()
>  			new Lexer(NULL, str->makeSubStream(str->getPos(), gFalse, 0, &parseObj)),
>  			gTrue
>  			);
> -		parser.getObj(&obj);
> +		if(!parser.getObj(&obj))
> +		{
> +			kernelPrintDbg(DBG_ERR, "Unable to parse stream");
> +			throw MalformedFormatExeption("parse stream");
> +		}
>  		if(obj.isCmd(XREF_KEYWORD))
>  		{
>  			obj.free();
> Index: src/tests/kernel/testccontentstream.cc
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/tests/kernel/testccontentstream.cc,v
> retrieving revision 1.7
> diff -u -p -r1.7 testccontentstream.cc
> --- src/tests/kernel/testccontentstream.cc	22 Feb 2008 08:42:08 -0000	1.7
> +++ src/tests/kernel/testccontentstream.cc	4 Mar 2008 20:12:45 -0000
> @@ -193,7 +193,7 @@ namespace  {
>  		  dict.initDict (xref);
>  		  o.free ();
>  		  
> -		  parser->getObj(&o);
> +		  parser->getObj(&o); // FIXME handle return value
>  		  while (!o.isCmd("ID") && !o.isEOF()) 
>  		  {
>  			if (!o.isName()) 
> @@ -205,7 +205,7 @@ namespace  {
>  			{
>  			  char* key = copyString(o.getName());
>  			  o.free();
> -			  parser->getObj(&o);
> +			  parser->getObj(&o); // FIXME handle return value
>  			  if (o.isEOF() || o.isError()) 
>  			  {
>  				gfree (key);
> @@ -214,7 +214,7 @@ namespace  {
>  			  dict.dictAdd(key, &o);
>  			}
>  			
> -			parser->getObj(&o);
> +			parser->getObj(&o); // FIXME handle return value
>  		  }
>  		 
>  		  
> @@ -553,7 +553,7 @@ primitiveprintContentStream (__attribute
>  			);
>  	
>  	Object o;
> -	parser->getObj (&o);
> +	parser->getObj (&o); // FIXME handle return value
>  	int i = 0;
>  	
>  	while (!o.isEOF()) 
> @@ -563,7 +563,7 @@ primitiveprintContentStream (__attribute
>  		{
>  			if (!img (parser.get(),o,xref))
>  				return false;
> -	    	parser->getObj(&o);
> +	    	parser->getObj(&o); // FIXME handle return value
>  		}
>  		
>  		if (o.isCmd ())
> @@ -580,7 +580,7 @@ primitiveprintContentStream (__attribute
>  
>  		// grab the next object
>  		o.free ();
> -		parser->getObj(&o);
> +		parser->getObj(&o); // FIXME handle return value
>  	}
>  
>  	obj.free ();
> @@ -664,7 +664,7 @@ cstreamsreader (__attribute__((unused))	
>  					)
>  				);
>  		Object o1;
> -		parser->getObj (&o1);
> +		parser->getObj (&o1); // FIXME handle return value
>  		///
>  		obj.free ();
>  
> @@ -686,7 +686,7 @@ cstreamsreader (__attribute__((unused))	
>  			reader.getXpdfObject (o);
>  		
>  			o1.free();
> -			parser->getObj (&o1);
> +			parser->getObj (&o1); // FIXME handle return value
>  
>  		}
>  		reader.close ();
> Index: src/tests/kernel/testmain.h
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/tests/kernel/testmain.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 testmain.h
> --- src/tests/kernel/testmain.h	22 Feb 2008 08:42:08 -0000	1.7
> +++ src/tests/kernel/testmain.h	4 Mar 2008 20:12:45 -0000
> @@ -159,13 +159,13 @@ print (__attribute__((unused)) ostream& 
>  				gTrue  // TODO is gtrue ok here?
>  				)
>  			);
> -	parser->getObj(&obj);
> +	parser->getObj(&obj); // FIXME handle return value
>  	while (!obj.isEOF()) 
>  	{
>  		//os << &obj;
>  		obj.free ();
>  		// grab the next object
> -		parser->getObj(&obj);
> +		parser->getObj(&obj); // FIXME handle return value
>  	}
>  }
>  
> Index: src/xpdf/xpdf/Gfx.cc
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/xpdf/xpdf/Gfx.cc,v
> retrieving revision 1.4
> diff -u -p -r1.4 Gfx.cc
> --- src/xpdf/xpdf/Gfx.cc	16 Jan 2008 13:50:36 -0000	1.4
> +++ src/xpdf/xpdf/Gfx.cc	4 Mar 2008 20:12:46 -0000
> @@ -550,12 +550,13 @@ void Gfx::display(Object *obj, GBool top
>      return;
>    }
>    parser = new Parser(xref, new Lexer(xref, obj), gFalse);
> -  go(topLevel);
> +  if(go(topLevel)==-1)
> +    error(-1, "Data corrupted");
>    delete parser;
>    parser = NULL;
>  }
>  
> -void Gfx::go(GBool topLevel) {
> +int Gfx::go(GBool topLevel) {
>    Object obj;
>    Object args[maxArgs];
>    int numArgs, i;
> @@ -564,7 +565,8 @@ void Gfx::go(GBool topLevel) {
>    // scan a sequence of objects
>    updateLevel = lastAbortCheck = 0;
>    numArgs = 0;
> -  parser->getObj(&obj);
> +  if(!parser->getObj(&obj))
> +    goto malformedErr;
>    while (!obj.isEOF()) {
>  
>      // got a command - execute it
> @@ -617,7 +619,8 @@ void Gfx::go(GBool topLevel) {
>      }
>  
>      // grab the next object
> -    parser->getObj(&obj);
> +    if (!parser->getObj(&obj))
> +      goto malformedErr;
>    }
>    obj.free();
>  
> @@ -641,6 +644,11 @@ void Gfx::go(GBool topLevel) {
>    if (topLevel && updateLevel > 0) {
>      out->dump();
>    }
> +  return 0;
> +
> +malformedErr:
> +  error(-1, "Cannot parse object. Malformed stream");
> +  return -1;
>  }
>  
>  void Gfx::execOp(Object *cmd, Object args[], int numArgs) {
> @@ -3895,22 +3903,28 @@ Stream *Gfx::buildImageStream() {
>  
>    // build dictionary
>    dict.initDict(xref);
> -  parser->getObj(&obj);
> +  if (!parser->getObj(&obj)) 
> +    goto malformedErr;
>    while (!obj.isCmd("ID") && !obj.isEOF()) {
>      if (!obj.isName()) {
>        error(getPos(), "Inline image dictionary key must be a name object");
>        obj.free();
> +      goto malformedErr;
>      } else {
>        key = copyString(obj.getName());
>        obj.free();
> -      parser->getObj(&obj);
> +      if (!parser->getObj(&obj)) {
> +        gfree(key);
> +	goto malformedErr;
> +      }
>        if (obj.isEOF() || obj.isError()) {
>  	gfree(key);
>  	break;
>        }
>        dict.dictAdd(key, &obj);
>      }
> -    parser->getObj(&obj);
> +    if (!parser->getObj(&obj))
> +      goto malformedErr;
>    }
>    if (obj.isEOF()) {
>      error(getPos(), "End of file in inline image");
> @@ -3922,9 +3936,14 @@ Stream *Gfx::buildImageStream() {
>  
>    // make stream
>    str = new EmbedStream(parser->getStream(), &dict, gFalse, 0);
> -  str = str->addFilters(&dict);
> +  if (str)
> +    str = str->addFilters(&dict);
>  
>    return str;
> +
> +malformedErr:
> +  error(-1, "Malformed content. Unable to parse inline image");
> +  return NULL;
>  }
>  
>  void Gfx::opImageData(Object args[], int numArgs) {
> Index: src/xpdf/xpdf/Gfx.h
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/xpdf/xpdf/Gfx.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 Gfx.h
> --- src/xpdf/xpdf/Gfx.h	16 Jan 2008 13:50:36 -0000	1.4
> +++ src/xpdf/xpdf/Gfx.h	4 Mar 2008 20:12:46 -0000
> @@ -4,6 +4,10 @@
>  //
>  // Copyright 1996-2003 Glyph & Cog, LLC
>  //
> +// Changes:
> +// Michal Hocko	- go() method has int instead of void return value and 
> +// 		  returns -1 if parsing error occurs.
> +//
>  //========================================================================
>  
>  #ifndef GFX_H
> @@ -166,7 +170,11 @@ private:
>  
>    static Operator opTab[];	// table of operators
>  
> -  void go(GBool topLevel);
> +  // returns -1 on error, 0 otherwise
> +  // force gcc to warn if return value is not checked FIXME remove after
> +  // tested
> +  int go(GBool topLevel) __attribute__((warn_unused_result)); 
> +
>    void execOp(Object *cmd, Object args[], int numArgs);
>    Operator *findOp(char *name);
>    GBool checkArg(Object *arg, TchkType type);
> Index: src/xpdf/xpdf/PDFDoc.cc
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/xpdf/xpdf/PDFDoc.cc,v
> retrieving revision 1.3
> diff -u -p -r1.3 PDFDoc.cc
> --- src/xpdf/xpdf/PDFDoc.cc	16 Jan 2008 13:50:36 -0000	1.3
> +++ src/xpdf/xpdf/PDFDoc.cc	4 Mar 2008 20:12:46 -0000
> @@ -353,6 +353,7 @@ void PDFDoc::processLinks(OutputDev *out
>    catalog->getPage(page)->processLinks(out, catalog);
>  }
>  
> +/*
>  GBool PDFDoc::isLinearized() {
>    Parser *parser;
>    Object obj1, obj2, obj3, obj4, obj5;
> @@ -383,6 +384,7 @@ GBool PDFDoc::isLinearized() {
>    delete parser;
>    return lin;
>  }
> +*/
>  
>  GBool PDFDoc::saveAs(GString *name) {
>    FILE *f;
> Index: src/xpdf/xpdf/PDFDoc.h
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/xpdf/xpdf/PDFDoc.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 PDFDoc.h
> --- src/xpdf/xpdf/PDFDoc.h	16 Jan 2008 13:50:36 -0000	1.3
> +++ src/xpdf/xpdf/PDFDoc.h	4 Mar 2008 20:12:46 -0000
> @@ -142,7 +142,7 @@ public:
>      { return xref->okToAddNotes(ignoreOwnerPW); }
>  
>    // Is this document linearized?
> -  GBool isLinearized();
> +  //GBool isLinearized();
>  
>    // Return the document's Info dictionary (if any).
>    Object *getDocInfo(Object *obj) { return xref->getDocInfo(obj); }
> Index: src/xpdf/xpdf/Parser.cc
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/xpdf/xpdf/Parser.cc,v
> retrieving revision 1.4
> diff -u -p -r1.4 Parser.cc
> --- src/xpdf/xpdf/Parser.cc	16 Jan 2008 13:50:36 -0000	1.4
> +++ src/xpdf/xpdf/Parser.cc	4 Mar 2008 20:12:46 -0000
> @@ -61,11 +61,19 @@ Object *Parser::getObj(Object *obj, Guch
>    if (buf1.isCmd("[")) {
>      shift();
>      obj->initArray(xref);
> -    while (!buf1.isCmd("]") && !buf1.isEOF())
> -      obj->arrayAdd(getObj(&obj2, fileKey, encAlgorithm, keyLength,
> -			   objNum, objGen));
> -    if (buf1.isEOF())
> +    while (!buf1.isCmd("]") && !buf1.isEOF()) {
> +      Object * child = getObj(&obj2, fileKey, encAlgorithm, keyLength,
> +			   objNum, objGen);
> +      if(!child) {
> +      	// FIXME free obj2?
> +	goto err;
> +      }
> +      obj->arrayAdd(child);
> +    }
> +    if (buf1.isEOF()) {
>        error(getPos(), "End of file inside array");
> +      goto err;
> +    }
>      shift();
>  
>    // dictionary or stream
> @@ -75,20 +83,29 @@ Object *Parser::getObj(Object *obj, Guch
>      while (!buf1.isCmd(">>") && !buf1.isEOF()) {
>        if (!buf1.isName()) {
>  	error(getPos(), "Dictionary key must be a name object");
> -	shift();
> -      } else {
> -	key = copyString(buf1.getName());
> -	shift();
> -	if (buf1.isEOF() || buf1.isError()) {
> -	  gfree(key);
> -	  break;
> -	}
> -	obj->dictAdd(key, getObj(&obj2, fileKey, encAlgorithm, keyLength,
> -				 objNum, objGen));
> +	buf1.free();
> +	goto err;
>        }
> +
> +      key = copyString(buf1.getName());
> +      shift();
> +      if (buf1.isEOF() || buf1.isError()) {
> +        gfree(key);
> +        break;
> +      }
> +      Object * child = getObj(&obj2, fileKey, encAlgorithm, keyLength,
> +      			 objNum, objGen);
> +      if(!child) {
> +        // FIXME free obj2?
> +        goto err;
> +      }
> +      obj->dictAdd(key, child);
>      }
> -    if (buf1.isEOF())
> +    if (buf1.isEOF()) {
>        error(getPos(), "End of file inside dictionary");
> +      buf1.free();
> +      goto err;
> +    }
>      // stream objects are not allowed inside content streams or
>      // object streams
>      if (allowStreams && buf2.isCmd("stream")) {
> @@ -139,6 +156,10 @@ Object *Parser::getObj(Object *obj, Guch
>    }
>  
>    return obj;
> +err:
> +  if(obj)
> +	  obj->free();
> +  return NULL;
>  }
>  
>  Stream *Parser::makeStream(Object *dict, Guchar *fileKey,
> @@ -226,7 +247,7 @@ void Parser::shift() {
>    else
>    {
>  	size_t pos = lexer->strIndex ();
> -    lexer->getObj(&buf2);
> +	lexer->getObj(&buf2);
>  	if (pos != lexer->strIndex())
>  		endOfActStream = 2;
>  	else if (0 < endOfActStream)
> Index: src/xpdf/xpdf/Parser.h
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/xpdf/xpdf/Parser.h,v
> retrieving revision 1.6
> diff -u -p -r1.6 Parser.h
> --- src/xpdf/xpdf/Parser.h	16 Jan 2008 13:50:36 -0000	1.6
> +++ src/xpdf/xpdf/Parser.h	4 Mar 2008 20:12:46 -0000
> @@ -31,9 +31,10 @@ public:
>    ~Parser();
>  
>    // Get the next object from the input stream.
> +  // Returns pointer to the given obj or NULL if not able to parse.
>    Object *getObj(Object *obj, Guchar *fileKey = NULL,
>  		 CryptAlgorithm encAlgorithm = cryptRC4, int keyLength = 0,
> -		 int objNum = 0, int objGen = 0);
> +		 int objNum = 0, int objGen = 0)__attribute__((warn_unused_result));
>  
>    // Get stream.
>    Stream *getStream() { return lexer->getStream(); }
> Index: src/xpdf/xpdf/XRef.cc
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/xpdf/xpdf/XRef.cc,v
> retrieving revision 1.12
> diff -u -p -r1.12 XRef.cc
> --- src/xpdf/xpdf/XRef.cc	2 Mar 2008 12:23:42 -0000	1.12
> +++ src/xpdf/xpdf/XRef.cc	4 Mar 2008 20:12:46 -0000
> @@ -115,8 +115,12 @@ ObjectStream::ObjectStream(XRef *xref, i
>    str = new EmbedStream(objStr.getStream(), &obj1, gTrue, first);
>    parser = new Parser(xref, new Lexer(xref, str), gFalse);
>    for (i = 0; i < nObjects; ++i) {
> -    parser->getObj(&obj1);
> -    parser->getObj(&obj2);
> +    if(!parser->getObj(&obj1))
> +      goto malformedErr;
> +    if(!parser->getObj(&obj2)) {
> +      obj1.free();
> +      goto malformedErr;
> +    }
>      if (!obj1.isInt() || !obj2.isInt()) {
>        obj1.free();
>        obj2.free();
> @@ -155,7 +159,9 @@ ObjectStream::ObjectStream(XRef *xref, i
>  			    offsets[i+1] - offsets[i]);
>      }
>      parser = new Parser(xref, new Lexer(xref, str), gFalse);
> -    parser->getObj(&objs[i]);
> +    if(!parser->getObj(&objs[i])) 
> +      goto malformedErr;
> +    
>      while (str->getChar() != EOF) ;
>      delete parser;
>    }
> @@ -165,6 +171,10 @@ ObjectStream::ObjectStream(XRef *xref, i
>   err1:
>    objStr.free();
>    return;
> + malformedErr:
> +  if(parser)
> +    delete parser;
> +  // TODO getObj how to hande ObjectStream constructor
>  }
>  
>  ObjectStream::~ObjectStream() {
> @@ -194,11 +204,18 @@ XRef::XRef(BaseStream *strA) {
>    // inits stream and initializes internals
>    str = strA;
>    // gets position of last xref section
> +  setErrCode(errNone);
>    Guint pos = getStartXref();
> -  if(errCode == errNone)
> +  if(isOk())
>      initInternals(pos);
>  }
>  
> +void XRef::setErrCode(int err)
> +{
> +  errCode = err;
> +  ok = (errCode == errNone)?gTrue:gFalse;
> +}
> +
>  /** Initializes all XRef internal structures.
>   * @param pos Position of xref table.
>   *
> @@ -209,7 +226,7 @@ void XRef::initInternals(Guint pos)
>    Object obj;
>  
>    ok = gTrue;
> -  errCode = errNone;
> +  setErrCode(errNone);
>    size = 0;
>    entries = NULL;
>    streamEnds = NULL;
> @@ -227,7 +244,7 @@ void XRef::initInternals(Guint pos)
>    // reconstruct the xref table
>    if (pos == 0) {
>      if (!(ok = constructXRef())) {
> -      errCode = errDamaged;
> +      setErrCode(errDamaged);
>        return;
>      }
>  
> @@ -239,7 +256,7 @@ void XRef::initInternals(Guint pos)
>      // try to reconstruct it
>      if (!ok) {
>        if (!(ok = constructXRef())) {
> -	errCode = errDamaged;
> +	setErrCode(errDamaged);
>  	return;
>        }
>      }
> @@ -254,7 +271,7 @@ void XRef::initInternals(Guint pos)
>    } else {
>      obj.free();
>      if (!(ok = constructXRef())) {
> -      errCode = errDamaged;
> +      setErrCode(errDamaged);
>        return;
>      }
>    }
> @@ -288,7 +305,7 @@ Guint XRef::getStartXref() {
>    char buf[xrefSearchSize+1];
>    int c, n, i;
>  
> -  errCode = errNone;
> +  setErrCode(errNone);
>  
>    // read last xrefSearchSize bytes
>    str->setPos(xrefSearchSize, -1);
> @@ -307,7 +324,7 @@ Guint XRef::getStartXref() {
>      }
>    }
>    if (i < 0) {
> -    errCode = errDamaged;
> +    setErrCode(errDamaged);
>      return 0;
>    }
>  
> @@ -334,7 +351,7 @@ Guint XRef::getStartXref() {
>  // Read one xref table section.  Also reads the associated trailer
>  // dictionary, and returns the prev pointer (if any).
>  GBool XRef::readXRef(Guint *pos) {
> -  Parser *parser;
> +  Parser *parser = NULL;
>    Object obj;
>    GBool more;
>  
> @@ -344,7 +361,8 @@ GBool XRef::readXRef(Guint *pos) {
>  	     new Lexer(NULL,
>  	       str->makeSubStream(start + *pos, gFalse, 0, &obj)),
>  	     gTrue);
> -  parser->getObj(&obj);
> +  if (!parser->getObj(&obj))
> +    goto malformedErr;
>  
>    // parse an old-style xref table
>    if (obj.isCmd("xref")) {
> @@ -354,17 +372,21 @@ GBool XRef::readXRef(Guint *pos) {
>    // parse an xref stream
>    } else if (obj.isInt()) {
>      obj.free();
> -    if (!parser->getObj(&obj)->isInt()) {
> +    Object * fetch;
> +    if (!(fetch = parser->getObj(&obj)))
> +      goto malformedErr;
> +    if (!fetch->isInt()) 
>        goto err1;
> -    }
>      obj.free();
> -    if (!parser->getObj(&obj)->isCmd("obj")) {
> +    if (!(fetch = parser->getObj(&obj)))
> +      goto malformedErr;
> +    if (!fetch->isCmd("obj")) 
>        goto err1;
> -    }
>      obj.free();
> -    if (!parser->getObj(&obj)->isStream()) {
> +    if (!(fetch = parser->getObj(&obj)))
> +      goto malformedErr;
> +    if (!fetch->isStream()) 
>        goto err1;
> -    }
>      more = readXRefStream(obj.getStream(), pos);
>      obj.free();
>  
> @@ -375,9 +397,11 @@ GBool XRef::readXRef(Guint *pos) {
>    delete parser;
>    return more;
>  
> - err1:
> +err1:
>    obj.free();
> -  delete parser;
> +malformedErr:
> +  if(parser)
> +    delete parser;
>    ok = gFalse;
>    return gFalse;
>  }
> @@ -387,10 +411,12 @@ GBool XRef::readXRefTable(Parser *parser
>    GBool more;
>    Object obj, obj2;
>    Guint pos2;
> +  Object * fetch;
>    int first, n, newSize, i;
>  
>    while (1) {
> -    parser->getObj(&obj);
> +    if (!parser->getObj(&obj))
> +      goto malformedErr;
>      if (obj.isCmd("trailer")) {
>        obj.free();
>        break;
> @@ -400,7 +426,9 @@ GBool XRef::readXRefTable(Parser *parser
>      }
>      first = obj.getInt();
>      obj.free();
> -    if (!parser->getObj(&obj)->isInt()) {
> +    if (!(fetch = parser->getObj(&obj)))
> +      goto malformedErr;
> +    if (!fetch->isInt()) {
>        goto err1;
>      }
>      n = obj.getInt();
> @@ -423,17 +451,22 @@ GBool XRef::readXRefTable(Parser *parser
>        size = newSize;
>      }
>      for (i = first; i < first + n; ++i) {
> -      if (!parser->getObj(&obj)->isInt()) {
> +      if (!(fetch = parser->getObj(&obj)))
> +        goto malformedErr;
> +      if (!fetch->isInt()) {
>  	goto err1;
>        }
>        entry.offset = (Guint)obj.getInt();
>        obj.free();
> -      if (!parser->getObj(&obj)->isInt()) {
> +      if (!(fetch = parser->getObj(&obj)))
> +        goto malformedErr;
> +      if (!fetch->isInt()) {
>  	goto err1;
>        }
>        entry.gen = obj.getInt();
>        obj.free();
> -      parser->getObj(&obj);
> +      if (!parser->getObj(&obj))
> +        goto malformedErr;
>        if (obj.isCmd("n")) {
>  	entry.type = xrefEntryUncompressed;
>        } else if (obj.isCmd("f")) {
> @@ -459,7 +492,9 @@ GBool XRef::readXRefTable(Parser *parser
>    }
>  
>    // read the trailer dictionary
> -  if (!parser->getObj(&obj)->isDict()) {
> +  if (!(fetch = parser->getObj(&obj)))
> +    goto malformedErr;
> +  if (!fetch->isDict()) {
>      goto err1;
>    }
>  
> @@ -497,8 +532,9 @@ GBool XRef::readXRefTable(Parser *parser
>    obj.free();
>    return more;
>  
> - err1:
> +err1:
>    obj.free();
> +malformedErr:
>    ok = gFalse;
>    return gFalse;
>  }
> @@ -669,7 +705,7 @@ GBool XRef::readXRefStreamSection(Stream
>  
>  // Attempt to construct an xref table for a damaged file.
>  GBool XRef::constructXRef() {
> -  Parser *parser;
> +  Parser *parser = NULL;
>    Object newTrailerDict, obj;
>    char buf[256];
>    Guint pos;
> @@ -706,7 +742,8 @@ GBool XRef::constructXRef() {
>  		 new Lexer(NULL,
>  		   str->makeSubStream(pos + 7, gFalse, 0, &obj)),
>  		 gFalse);
> -      parser->getObj(&newTrailerDict);
> +      if (!parser->getObj(&newTrailerDict))
> +        goto malformedErr;
>        if (newTrailerDict.isDict()) {
>  	newTrailerDict.dictLookupNF("Root", &obj);
>  	if (obj.isRef()) {
> @@ -785,6 +822,11 @@ GBool XRef::constructXRef() {
>  
>    error(-1, "Couldn't find trailer dictionary");
>    return gFalse;
> +malformedErr:
> +  error(-1, "malformed content. Not able to parse.");
> +  if (parser)
> +    delete parser;
> +  return gFalse;
>  }
>  
>  void XRef::setEncryption(int permFlagsA, GBool ownerPasswordOkA,
> @@ -827,12 +869,14 @@ Object *XRef::fetch(int num, int gen, Ob
>    XRefEntry *e;
>    Parser *parser;
>    Object obj1, obj2, obj3;
> +  GBool failed = gFalse;
>  
>    // check for bogus ref - this can happen in corrupted PDF files
>    if (num < 0 || num >= size) {
>      goto err;
>    }
>  
> +  setErrCode(errNone);
>    e = &entries[num];
>    switch (e->type) {
>  
> @@ -845,9 +889,16 @@ Object *XRef::fetch(int num, int gen, Ob
>  	       new Lexer(this,
>  		 str->makeSubStream(start + e->offset, gFalse, 0, &obj1)),
>  	       gTrue);
> -    parser->getObj(&obj1);
> -    parser->getObj(&obj2);
> -    parser->getObj(&obj3);
> +    if (!parser->getObj(&obj1))
> +      goto err;
> +    if (!parser->getObj(&obj2)) {
> +      obj1.free();
> +      goto err;
> +    }
> +    if (!parser->getObj(&obj3)) {
> +      obj2.free();
> +      goto err;
> +    } 
>      if (!obj1.isInt() || obj1.getInt() != num ||
>  	!obj2.isInt() || obj2.getInt() != gen ||
>  	!obj3.isCmd("obj")) {
> @@ -857,12 +908,16 @@ Object *XRef::fetch(int num, int gen, Ob
>        delete parser;
>        goto err;
>      }
> -    parser->getObj(obj, encrypted ? fileKey : (Guchar *)NULL,
> -		   encAlgorithm, keyLength, num, gen);
> +    if (!parser->getObj(obj, encrypted ? fileKey : (Guchar *)NULL,
> +		   encAlgorithm, keyLength, num, gen)) 
> +      failed = gTrue;
> +
>      obj1.free();
>      obj2.free();
>      obj3.free();
>      delete parser;
> +    if (failed)
> +      goto err;
>      break;
>  
>    case xrefEntryCompressed:
> @@ -885,6 +940,8 @@ Object *XRef::fetch(int num, int gen, Ob
>    return obj;
>  
>   err:
> +  setErrCode(errDamaged);
> +  ok = false;
>    return obj->initNull();
>  }
>  
> Index: src/xpdf/xpdf/XRef.h
> ===================================================================
> RCS file: /cvsroot/pdfedit/pdfedit/src/xpdf/xpdf/XRef.h,v
> retrieving revision 1.10
> diff -u -p -r1.10 XRef.h
> --- src/xpdf/xpdf/XRef.h	16 Jan 2008 13:50:37 -0000	1.10
> +++ src/xpdf/xpdf/XRef.h	4 Mar 2008 20:12:46 -0000
> @@ -16,6 +16,7 @@
>  //              - getNumObjects rewriten to return just number of real objects
>  //                not size of entries array
>  //              - new knowsRef method
> +//              - setErrCode method added
>  //
>  //========================================================================
>  
> @@ -79,6 +80,10 @@ public:
>    // Is xref table valid?
>    virtual GBool isOk() { return ok; }
>  
> +  // sets error code and ok accordingly
> +  // if err == errNone then ok is set to true
> +  virtual void setErrCode(int err);
> +
>    // Get the error code (if isOk() returns false).
>    virtual int getErrorCode() { return errCode; }
>  

> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Pdfedit-devel mailing list
> Pdf...@li...
> https://lists.sourceforge.net/lists/listinfo/pdfedit-devel


-- 
Michal Hocko