Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TypeError that leads to an unhandled Exception #6

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

SebastianDeiss
Copy link

Hi @jbremer,

This pull request fixes jesparza#70.

I checked out your master and then cherry-picked my commits from https://github.com/SebastianDeiss/peepdf/tree/peepdf-issue-70-fix.

Sebastian Deiss added 2 commits March 6, 2018 09:34
A new optional parameter 'parser' was introduced for 'PDFObjectStream',
which takes a 'PDFParser' object.
This object is then used in 'resolveReferences()' to invoke
'PDFParser.readObject()'.
This commit fixes jesparza#70.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 24.764% when pulling 11e0c32 on SebastianDeiss:upstream-issue-70-fix into 12c7b19 on jbremer:master.

@Jack28
Copy link

Jack28 commented Apr 9, 2018

+1

@jbremer
Copy link
Member

jbremer commented Apr 9, 2018

Any sample that can be used for unit testing?

@SebastianDeiss
Copy link
Author

The issue here is a TypeError, which is the result of a piece of broken code in peepdf.
I am trying to create a document, which makes peepdf run that broken code, since the documents I used for testing cannot be made public.

@Jack28
Copy link

Jack28 commented Apr 20, 2018

It seems to be only eventbrite tickets:
Producer: PDFlib Personalization Server 8.0.4p2 (Python 2.5/Linux-x86_64)

michaelweiser added a commit to michaelweiser/peepdf that referenced this pull request Mar 6, 2020
With the previous change deferring reading of objects from the decoded
stream until references can be resolved, it now runs into
jesparza#70. This change provides a different approach in fixing
it to hatching#6 by syncing it with the other locations where the identical code
is in use:

1. Force the numbers extracted by re.findall to int() as before,
   avoiding the TypeError exception:

Traceback (most recent call last):
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/main.py", line 409, in main
    ret, pdf = pdfParser.parse(fileName, options.isForceMode, options.isLooseMode, options.isManualAnalysis)
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 7117, in parse
    ret = body.updateObjects()
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 4291, in updateObjects
    object.resolveReferences()
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 3256, in resolveReferences
    ret = PDFParser.readObject(objectsSection[offset:])
TypeError: slice indices must be integers or None or have an __index__ method

2. Instantiate a new PDFParser object by adding the missing braces,
   avoiding another TypeError because readObject is no class method:

Traceback (most recent call last):
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/main.py", line 409, in main
    ret, pdf = pdfParser.parse(fileName, options.isForceMode, options.isLooseMode, options.isManualAnalysis)
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 7118, in parse
    ret = body.updateObjects()
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 4292, in updateObjects
    object.resolveReferences()
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 3256, in resolveReferences
    ret = PDFParser.readObject(objectsSection[offset:])
TypeError: unbound method readObject() must be called with PDFParser instance as first argument (got str instance instead)

3. Explicitly force the id to be an int() as well and append it do the
   list of indices as at the other callsites of this code. This solves
   no issue I have run into but seems sensible to avoid other potential
   TypeErrors and keep internal bookkeeping of the object consistent.

This should conclusively resolve jesparza#70 and supersedes hatching#6.

Signed-off-by: Michael Weiser <[email protected]>
@michaelweiser
Copy link

I can reproduce the error with sample files from https://www.infotek.co.jp/pdflib/demo/sample/, particularly https://www.infotek.co.jp/pdflib/demo/sample/image.php, which are /Producer (PDFlib Personalization Server 9.0.5p1 \(PHP5/Linux-x86_64\). Fixes are in commits 2 and 3 of #9 and should be more comprehensive (if I do say so myself).
The underlying problem seems to have been a missed update of duplicated code in the reference resolution method combined with a missing guard against trying to parse a buffer that had intentionally not been filled yet.
@SebastianDeiss: Can you have a look and give some feedback?

michelcrypt4d4mus added a commit to michelcrypt4d4mus/peepdf that referenced this pull request Aug 26, 2022
* Fix object stream parsing

Commit 8cc27b6 broke object stream parsing by resetting the content
cursor PDFParser.charCounter to zero on every invocation. This broke object
stream parsing. Reproducer:

$ echo -e "create pdf\ncreate object_stream\nall\nsave /tmp/foo.pdf" | \
	peepdf -i

Without fix:

$ peepdf -j /tmp/foo.pdf
Error: An error has occurred while parsing an indirect object!!

With this change: JSON output as expected (same for other outputs).

$ peepdf -j /tmp/foo.pdf
{
    "peepdf_analysis": {
[...]
           "version": "0.3"
        }
    }
}

Signed-off-by: Michael Weiser <[email protected]>

* Delay reading of objects until references are resolved

For unclear reasons, PDFObjectStream.update() delays decoding of the
modified raw stream until all references can be resolved. It does
however then go on to always try to extract objects from the still empty
decoded stream. This produces an error from peepdf cli:

$ peepdf image.php
Error: An error has occurred while parsing an indirect object!!

The error from PDFObjectStream.update() is "Missing offsets in object
stream" because self.decodedStream is still empty at that point, making
offsetsSection and eventually the numbers list empty, causing the abort.

This is triggered by /Length being a reference and setting updateNeeded
to True. Sample: https://www.infotek.co.jp/pdflib/demo/sample/image.php.
Relevant PDF structure:

32 0 obj
<</Length 43 0 R/Filter/FlateDecode/Type/ObjStm/N 7/First 47>>
stream
[...]
endstream
endobj
43 0 obj
461
endobj

(Length in dict of object 32 R-eferences object 43 which contains 461
what presumably is the length of the stream - which does not seem to be
used or checked for consistency by peepdf atm, btw.)

This resolves the first half of jesparza#70 in that force mode is
no longer necessary to parse such files at all.

Signed-off-by: Michael Weiser <[email protected]>

* Avoid TypeError on reference resolution

With the previous change deferring reading of objects from the decoded
stream until references can be resolved, it now runs into
jesparza#70. This change provides a different approach in fixing
it to hatching#6 by syncing it with the other locations where the identical code
is in use:

1. Force the numbers extracted by re.findall to int() as before,
   avoiding the TypeError exception:

Traceback (most recent call last):
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/main.py", line 409, in main
    ret, pdf = pdfParser.parse(fileName, options.isForceMode, options.isLooseMode, options.isManualAnalysis)
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 7117, in parse
    ret = body.updateObjects()
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 4291, in updateObjects
    object.resolveReferences()
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 3256, in resolveReferences
    ret = PDFParser.readObject(objectsSection[offset:])
TypeError: slice indices must be integers or None or have an __index__ method

2. Instantiate a new PDFParser object by adding the missing braces,
   avoiding another TypeError because readObject is no class method:

Traceback (most recent call last):
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/main.py", line 409, in main
    ret, pdf = pdfParser.parse(fileName, options.isForceMode, options.isLooseMode, options.isManualAnalysis)
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 7118, in parse
    ret = body.updateObjects()
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 4292, in updateObjects
    object.resolveReferences()
  File "peepdf-venv2/lib64/python2.7/site-packages/peepdf/PDFCore.py", line 3256, in resolveReferences
    ret = PDFParser.readObject(objectsSection[offset:])
TypeError: unbound method readObject() must be called with PDFParser instance as first argument (got str instance instead)

3. Explicitly force the id to be an int() as well and append it do the
   list of indices as at the other callsites of this code. This solves
   no issue I have run into but seems sensible to avoid other potential
   TypeErrors and keep internal bookkeeping of the object consistent.

This should conclusively resolve jesparza#70 and supersedes hatching#6.

Signed-off-by: Michael Weiser <[email protected]>

* Fix PDFParser.readSymbol(), if while-space characters precede the symbol

In PDF files the Cross-Reference Table or a Cross-Reference Stream
contain byte-offsets for the start of objects within the file or the
uncompressed stream. Such an offset does not always point the first byte
of the initial token (see ISO 32000-2008 section 7.2.2) of the referenced
object. The object may be preceded by white-space characters and comments.

Without this commit PDFParser.readSymbol() fails to read a symbol, if the
first character to be processed is a white-space character. This commit
changes PDFParser.readSymbol() to skip leading white-space characters.
(PDFParser.readSymbol() already skips any number of leading comments followed
by white-space characters.) This enables passing of PDF-files with sloppy
cross reference offsets.

* Add the missing method PDFArray.getJSCode()

An object of class PDFArray can contain JS-code, if one or more array-elements
contain JS-code. The getter method was simply missing.

* Handle sloppy cross references more and less generically

A previous commit adjusted readSymbol() to skip leading whitespace in
order to avoid errors with sloppy cross references. This did not fix
handling of literals such as numbers and booleans in readObject()
because they're not accessed using readSymbol(). Also, adjusting the
very low-level readSymbol() function might generate fallout.

So instead, this change moves the skipping of leading whitespace into
readObject() so that it affects all types of referenced objects equally
but not all symbol lookups altogether.

Signed-off-by: Michael Weiser <[email protected]>

Signed-off-by: Michael Weiser <[email protected]>
Co-authored-by: Michael Weiser <[email protected]>
Co-authored-by: Anselm Kruis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError leads to an unhandled Exception
5 participants