Fix an infinite loop parsing in CPDF_SyntaxParser::GetObject()

Fix an infinite loop parsing in CPDF_SyntaxParser::GetObject()

CPDF_SyntaxParser::GetObject() may enter into an infinite loop when a
signature dictionary doesn't have 'Contents' field. Add a check to
avoid that.

BUG=pdfium:344
R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/d3ab0f383f6736657480a8bb418c5e715a1aed3b

[Wei Li, 2016-01-11 19:15:58]

Description was changed from

==========
Fix an infinit loop parsing in GetObject()

CPDF_SyntaxParser::GetObject() may enter into an infinite loop when a
signature dictionary doesn't have 'Contents' field. Add a check to
avoid that.

BUG=pdfium:344
==========

to

==========
Fix an infinit loop parsing in CPDF_SyntaxParser::GetObject()

CPDF_SyntaxParser::GetObject() may enter into an infinite loop when a
signature dictionary doesn't have 'Contents' field. Add a check to
avoid that.

BUG=pdfium:344
==========

[Wei Li, 2016-01-11 19:15:58]

weili@chromium.org changed reviewers:
+ thestig@chromium.org

[Wei Li, 2016-01-11 19:16:16]

PTAL, thanks

[Lei Zhang, 2016-01-11 19:50:27]

Description was changed from

==========
Fix an infinit loop parsing in CPDF_SyntaxParser::GetObject()

CPDF_SyntaxParser::GetObject() may enter into an infinite loop when a
signature dictionary doesn't have 'Contents' field. Add a check to
avoid that.

BUG=pdfium:344
==========

to

==========
Fix an infinite loop parsing in CPDF_SyntaxParser::GetObject()

CPDF_SyntaxParser::GetObject() may enter into an infinite loop when a
signature dictionary doesn't have 'Contents' field. Add a check to
avoid that.

BUG=pdfium:344
==========

[Lei Zhang, 2016-01-11 19:55:03]

https://codereview.chromium.org/1575833004/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1575833004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:2168: if
(IsSignatureDict(pDict.get()) && dwSignValuePos) {
If |dwSignValuePos| is non-zero, but still invalid, can we still enter an
infinite loop?

https://codereview.chromium.org/1575833004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:2169: FX_FILESIZE dwSavePos
= m_Pos;
Replace with CFX_AutoRestorer while we are here?

https://codereview.chromium.org/1575833004/diff/1/fpdfsdk/src/fpdfview_embedd...
File fpdfsdk/src/fpdfview_embeddertest.cpp (right):

https://codereview.chromium.org/1575833004/diff/1/fpdfsdk/src/fpdfview_embedd...
fpdfsdk/src/fpdfview_embeddertest.cpp:226: // The test should pass when circular
references to ParseIndirectObject will not
Does this description need to be updated?

[Wei Li, 2016-01-11 21:07:20]

https://codereview.chromium.org/1575833004/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1575833004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:2168: if
(IsSignatureDict(pDict.get()) && dwSignValuePos) {
Infinite loops caused during parsing should repeatedly try to parse objects at
the same position. |dwSignValuePos| is assigned to the actual file position of
'/Contents' field (line 2156), so it won't be arbitrary and should not point
backwards to cause loop.

https://codereview.chromium.org/1575833004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:2169: FX_FILESIZE dwSavePos
= m_Pos;
On 2016/01/11 19:55:02, Lei Zhang wrote:
> Replace with CFX_AutoRestorer while we are here?

Done.

https://codereview.chromium.org/1575833004/diff/1/fpdfsdk/src/fpdfview_embedd...
File fpdfsdk/src/fpdfview_embeddertest.cpp (right):

https://codereview.chromium.org/1575833004/diff/1/fpdfsdk/src/fpdfview_embedd...
fpdfsdk/src/fpdfview_embeddertest.cpp:226: // The test should pass when circular
references to ParseIndirectObject will not
On 2016/01/11 19:55:03, Lei Zhang wrote:
> Does this description need to be updated?

Done.

[Lei Zhang, 2016-01-11 21:33:43]

lgtm

[Lei Zhang, 2016-01-11 21:35:13]

BTW, I fixed a typo in the commit msg.

[Wei Li, 2016-01-11 22:05:47]

Description was changed from

==========
Fix an infinite loop parsing in CPDF_SyntaxParser::GetObject()

CPDF_SyntaxParser::GetObject() may enter into an infinite loop when a
signature dictionary doesn't have 'Contents' field. Add a check to
avoid that.

BUG=pdfium:344
==========

to

==========
Fix an infinite loop parsing in CPDF_SyntaxParser::GetObject()

CPDF_SyntaxParser::GetObject() may enter into an infinite loop when a
signature dictionary doesn't have 'Contents' field. Add a check to
avoid that.

BUG=pdfium:344
R=thestig@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/d3ab0f383f6736657480a8bb418c5e715a1a...
==========

[Wei Li, 2016-01-11 22:05:48]

Committed patchset #3 (id:40001) manually as
d3ab0f383f6736657480a8bb418c5e715a1aed3b (presubmit successful).