diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-10-24 15:47:22 +0200 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2018-10-25 15:13:31 +0000 |
commit | 7a3ca41e453cbb702c1490649865228af205d948 (patch) | |
tree | 46e87bbf92b672f63dbe6b5a40111a5dd2b32274 | |
parent | 329c1780d9edc69fa84f8995711a3f25de153061 (diff) | |
download | qtwebengine-chromium-7a3ca41e453cbb702c1490649865228af205d948.tar.gz |
[Backport] Fix for CVE-2018-17469
M70: Validate decoder pipelines.
PDF decoders, AKA filters, can be chained together. There can be
an arbitrary number of decoding / decompressing filters in the pipeline,
but there should be at most 1 image decoder, and the image decoder
should only be at the end of the chain.
BUG=chromium:880675
TBR=tsepez@chromium.org
Change-Id: Iffa27c70ec1ed7574e38e0de23413840ee900959
Reviewed-on: https://pdfium-review.googlesource.com/42711
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
(cherry picked from commit 5f2ea0f6ef587f9f7a2fec9f80dbc82b94c97400)
Reviewed-on: https://pdfium-review.googlesource.com/42970
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Michael BrĂ¼ning <michael.bruning@qt.io>
3 files changed, 103 insertions, 0 deletions
diff --git a/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode.cpp b/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode.cpp index 9ccca121adc..9c4bdff205c 100644 --- a/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode.cpp +++ b/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode.cpp @@ -24,6 +24,7 @@ #include "core/fxcodec/fx_codec.h" #include "core/fxcrt/fx_extension.h" #include "third_party/base/numerics/safe_math.h" +#include "third_party/base/stl_util.h" namespace { @@ -79,6 +80,22 @@ const uint16_t PDFDocEncoding[256] = { 0x00f3, 0x00f4, 0x00f5, 0x00f6, 0x00f7, 0x00f8, 0x00f9, 0x00fa, 0x00fb, 0x00fc, 0x00fd, 0x00fe, 0x00ff}; +bool ValidateDecoderPipeline(const CPDF_Array* pDecoders) { + size_t count = pDecoders->GetCount(); + if (count <= 1) + return true; + + // TODO(thestig): Consolidate all the places that use these filter names. + static const char kValidDecoders[][16] = { + "FlateDecode", "Fl", "LZWDecode", "LZW", "ASCII85Decode", "A85", + "ASCIIHexDecode", "AHx", "RunLengthDecode", "RL"}; + for (size_t i = 0; i < count - 1; ++i) { + if (!pdfium::ContainsValue(kValidDecoders, pDecoders->GetStringAt(i))) + return false; + } + return true; +} + uint32_t A85Decode(const uint8_t* src_buf, uint32_t src_size, uint8_t** dest_buf, @@ -350,6 +367,9 @@ bool PDF_DataDecode(const uint8_t* src_buf, std::vector<std::pair<ByteString, const CPDF_Object*>> DecoderArray; if (const CPDF_Array* pDecoders = pDecoder->AsArray()) { + if (!ValidateDecoderPipeline(pDecoders)) + return false; + const CPDF_Array* pParamsArray = ToArray(pParams); for (size_t i = 0; i < pDecoders->GetCount(); ++i) { DecoderArray.push_back( diff --git a/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode.h b/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode.h index 328fd474c6b..a05cd561f99 100644 --- a/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode.h +++ b/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode.h @@ -13,11 +13,14 @@ #include "core/fxcrt/unowned_ptr.h" class CCodec_ScanlineDecoder; +class CPDF_Array; class CPDF_Dictionary; // Indexed by 8-bit char code, contains unicode code points. extern const uint16_t PDFDocEncoding[256]; +bool ValidateDecoderPipeline(const CPDF_Array* pDecoders); + ByteString PDF_EncodeString(const ByteString& src, bool bHex); WideString PDF_DecodeText(const uint8_t* pData, uint32_t size); WideString PDF_DecodeText(const ByteString& bstr); diff --git a/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp b/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp index 9ab49588252..e75ea1b3d3e 100644 --- a/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp +++ b/chromium/third_party/pdfium/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp @@ -4,9 +4,89 @@ #include "core/fpdfapi/parser/fpdf_parser_decode.h" +#include "core/fpdfapi/parser/cpdf_array.h" +#include "core/fpdfapi/parser/cpdf_name.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/test_support.h" +TEST(fpdf_parser_decode, ValidateDecoderPipeline) { + { + // Empty decoder list is always valid. + CPDF_Array decoders; + EXPECT_TRUE(ValidateDecoderPipeline(&decoders)); + } + { + // 1 decoder is always valid. + CPDF_Array decoders; + decoders.AddNew<CPDF_Name>("FlateEncode"); + EXPECT_TRUE(ValidateDecoderPipeline(&decoders)); + } + { + // 1 decoder is always valid, even with an unknown decoder. + CPDF_Array decoders; + decoders.AddNew<CPDF_Name>("FooBar"); + EXPECT_TRUE(ValidateDecoderPipeline(&decoders)); + } + { + // Valid 2 decoder pipeline. + CPDF_Array decoders; + decoders.AddNew<CPDF_Name>("AHx"); + decoders.AddNew<CPDF_Name>("LZWDecode"); + EXPECT_TRUE(ValidateDecoderPipeline(&decoders)); + } + { + // Valid 2 decoder pipeline. + CPDF_Array decoders; + decoders.AddNew<CPDF_Name>("ASCII85Decode"); + decoders.AddNew<CPDF_Name>("ASCII85Decode"); + EXPECT_TRUE(ValidateDecoderPipeline(&decoders)); + } + { + // Valid 5 decoder pipeline. + CPDF_Array decoders; + decoders.AddNew<CPDF_Name>("ASCII85Decode"); + decoders.AddNew<CPDF_Name>("A85"); + decoders.AddNew<CPDF_Name>("RunLengthDecode"); + decoders.AddNew<CPDF_Name>("FlateDecode"); + decoders.AddNew<CPDF_Name>("RL"); + EXPECT_TRUE(ValidateDecoderPipeline(&decoders)); + } + { + // Valid 5 decoder pipeline, with an image decoder at the end. + CPDF_Array decoders; + decoders.AddNew<CPDF_Name>("RunLengthDecode"); + decoders.AddNew<CPDF_Name>("ASCII85Decode"); + decoders.AddNew<CPDF_Name>("FlateDecode"); + decoders.AddNew<CPDF_Name>("LZW"); + decoders.AddNew<CPDF_Name>("DCTDecode"); + EXPECT_TRUE(ValidateDecoderPipeline(&decoders)); + } + { + // Invalid 2 decoder pipeline, with 2 image decoders. + CPDF_Array decoders; + decoders.AddNew<CPDF_Name>("DCTDecode"); + decoders.AddNew<CPDF_Name>("CCITTFaxDecode"); + EXPECT_FALSE(ValidateDecoderPipeline(&decoders)); + } + { + // Invalid 2 decoder pipeline, with 1 image decoder at the start. + CPDF_Array decoders; + decoders.AddNew<CPDF_Name>("DCTDecode"); + decoders.AddNew<CPDF_Name>("FlateDecode"); + EXPECT_FALSE(ValidateDecoderPipeline(&decoders)); + } + { + // Invalid 5 decoder pipeline. + CPDF_Array decoders; + decoders.AddNew<CPDF_Name>("FlateDecode"); + decoders.AddNew<CPDF_Name>("FlateDecode"); + decoders.AddNew<CPDF_Name>("DCTDecode"); + decoders.AddNew<CPDF_Name>("FlateDecode"); + decoders.AddNew<CPDF_Name>("FlateDecode"); + EXPECT_FALSE(ValidateDecoderPipeline(&decoders)); + } +} + TEST(fpdf_parser_decode, A85Decode) { pdfium::DecodeTestData test_data[] = { // Empty src string. |