rharrison: do you mind taking a look please?

This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/2bf05a6ca144e78223795ae1716875d3c9b8acb1

commit 2bf05a6ca144e78223795ae1716875d3c9b8acb1
Author: Ryan Harrison <rharrison@chromium.org>
Date: Tue Sep 05 16:20:37 2017

Leave space for null characters when getting text

The conversion from WideString to ByeString adds in null characters at
the end, so we need to account for these when selecting the range of
text to initially extract.

BUG= chromium:761770 , chromium:761626 

Change-Id: Ib8f863e997ebccaaf882e0beb29733f27a18826d
Reviewed-on: https://pdfium-review.googlesource.com/13110
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

[modify] https://crrev.com/2bf05a6ca144e78223795ae1716875d3c9b8acb1/fpdfsdk/fpdftext_embeddertest.cpp
[modify] https://crrev.com/2bf05a6ca144e78223795ae1716875d3c9b8acb1/fpdfsdk/fpdftext.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/add87b2d3b01f8cebcc9892bfecf7c90fbc1383b

commit add87b2d3b01f8cebcc9892bfecf7c90fbc1383b
Author: Ryan Harrison <rharrison@chromium.org>
Date: Tue Sep 05 20:25:25 2017

Account for string terminator added to FPDFText_GetText strings

The documentation for this function in PDFium explictly calls out that
the caller needs to pass in a buffer that is number of characters
desired + 1, to account for the terminator.

The existing implementation does not correctly do this, grabbing count
characters instead of count - 1 characters. This causes the written
out string to be count + 1 long due to an added terminator. The
corrected implementation cannot be rolled in current, since it causes
the strings being returned to be shorter then expected.

This CL changes the call sites in Chrome to follow the documentated
behaviour, which works correctly with both the old and new
implementation of the function. For the old implementation it will
overallocate by 1, and for the for the new implementation the
allocation size will be correct.

BUG= chromium:761770 , chromium:761626 

Change-Id: I874a88d854c2c85c28847ade3d74bcce56d6a876
Reviewed-on: https://chromium-review.googlesource.com/650650
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499732}
[modify] https://crrev.com/add87b2d3b01f8cebcc9892bfecf7c90fbc1383b/pdf/pdfium/pdfium_engine.cc
[modify] https://crrev.com/add87b2d3b01f8cebcc9892bfecf7c90fbc1383b/pdf/pdfium/pdfium_range.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32f8c89cd274ba200f8786eb0997439200f9cdf6

commit 32f8c89cd274ba200f8786eb0997439200f9cdf6
Author: pdfium-deps-roller@chromium.org <pdfium-deps-roller@chromium.org>
Date: Tue Sep 05 22:08:06 2017

Roll src/third_party/pdfium/ 47a90b894..067b78503 (31 commits)

https://pdfium.googlesource.com/pdfium.git/+log/47a90b894ecc..067b785037a5

$ git log 47a90b894..067b78503 --date=short --no-merges --format='%ad %ae %s'
2017-09-05 hnakashima Add unit tests for Code 39 barcode writer.
2017-09-05 dsinclair Rename fx_rand to fx_random
2017-09-05 hnakashima Remove isDevice parameter from barcode Render().
2017-09-05 dsinclair Cleanup CFX_WordBreak
2017-08-31 art-snake Remove redundant code.
2017-09-05 dsinclair Remove unused CFX_CharIter class
2017-09-05 rharrison Leave space for null characters when getting text
2017-09-05 dsinclair Split fx_guid from fx_extension
2017-09-05 dsinclair Split random code from fx_extension.h
2017-09-04 art-snake Fix length field in dictionary on create stream
2017-09-01 art-snake Refactoring of CPDF_IndirectObjectHolder.
2017-09-01 art-snake Move Parsing of indirect object logic into CPDF_SyntaxParser.
2017-09-01 npm Upgrade OpenJPEG to 2.2.0
2017-09-01 tsepez Split fx_ucd.h into fx_unicode.h and fx_ucddata.h.
2017-09-01 hnakashima Add unit tests for EAN-8 barcode writer.
2017-09-01 hnakashima Fix EAN-13 checksum and add unit tests.
2017-09-01 rharrison Cleanup usages of Mid(foo, 1), Right(1), and Left(1)
2017-09-01 rharrison Prepare for converting FX_STRSIZE int->size_t
2017-09-01 hnakashima Fix integer overflow in Buffer_itoa when passing INT_MIN.
2017-09-01 rharrison Adjust loops in preperation for FX_STRSIZE int->size_t
2017-08-31 hnakashima Remove unused FXFORMAT_HEX and FXFORMAT_CAPITAL.
2017-08-31 dsinclair Rename fxcrt_ and ifxcrt_ files to better names
2017-08-31 rharrison Make FPDF_GetText stricter on inputs
2017-08-31 thestig Change FPDFImageObj_GetImageFilter() to return byte strings.
2017-08-31 dsinclair Move implementations into fx_system.cpp
2017-08-31 dsinclair Move fx_extension implementation to cpp file
2017-08-31 dsinclair Move stream code into fx_stream.cpp
2017-08-31 art-snake Disable objects decryption if it is useless.
2017-08-31 dsinclair Move methods string methods to fx_string.cpp
2017-08-30 hnakashima Fix colorspace loading for mutually referencing colorspaces.
2017-08-31 dsinclair Cleanup fx_basic_* files

Created with:
  roll-dep src/third_party/pdfium
BUG= 761770 , 761626 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


TBR=dsinclair@chromium.org

Change-Id: I19bed919e2050fedbe6e323c92bb34271b31a2df
Reviewed-on: https://chromium-review.googlesource.com/651072
Reviewed-by: <pdfium-deps-roller@chromium.org>
Commit-Queue: <pdfium-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499763}
[modify] https://crrev.com/32f8c89cd274ba200f8786eb0997439200f9cdf6/DEPS

ClusterFuzz has detected this issue as fixed in range 501053:501100.

Detailed report: https://clusterfuzz.com/testcase?key=6041597883187200

Fuzzer: ifratric_acrojs
Job Type: windows_asan_chrome_no_sandbox
Platform Id: windows

Crash Type: Stack-buffer-overflow WRITE {*}
Crash Address: 0x0053c6a8
Crash State:
  FPDFText_GetText
  chrome_pdf::PDFiumRange::GetText
  chrome_pdf::PDFiumEngine::GetSelectedText
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=499331:499354
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=501053:501100

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6041597883187200

Additional requirements: Requires Gestures

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 6041597883187200 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e3efb73180eac56a29a867720d41a3ffa04e032

commit 0e3efb73180eac56a29a867720d41a3ffa04e032
Author: Ryan Harrison <rharrison@chromium.org>
Date: Tue Nov 28 17:30:36 2017

Revert "Account for string terminator added to FPDFText_GetText strings"

This reverts commit add87b2d3b01f8cebcc9892bfecf7c90fbc1383b.

Reason for revert:
This CL was correcting for an issue with the API function in PDFium.
https://pdfium-review.googlesource.com/c/pdfium/+/19350 corrects this
issue in PDFium, so this attempted work around is no longer needed. Once
the PDFium side change lands and is rolled into Chrome this will need to
be landed.

Original change's description:
> Account for string terminator added to FPDFText_GetText strings
>
> The documentation for this function in PDFium explictly calls out that
> the caller needs to pass in a buffer that is number of characters
> desired + 1, to account for the terminator.
>
> The existing implementation does not correctly do this, grabbing count
> characters instead of count - 1 characters. This causes the written
> out string to be count + 1 long due to an added terminator. The
> corrected implementation cannot be rolled in current, since it causes
> the strings being returned to be shorter then expected.
>
> This CL changes the call sites in Chrome to follow the documentated
> behaviour, which works correctly with both the old and new
> implementation of the function. For the old implementation it will
> overallocate by 1, and for the for the new implementation the
> allocation size will be correct.
>
> BUG= chromium:761770 , chromium:761626 
>
> Change-Id: I874a88d854c2c85c28847ade3d74bcce56d6a876
> Reviewed-on: https://chromium-review.googlesource.com/650650
> Reviewed-by: dsinclair <dsinclair@chromium.org>
> Commit-Queue: Ryan Harrison <rharrison@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#499732}

TBR=rharrison@chromium.org,dsinclair@chromium.org,paulmeyer@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:761770 ,  chromium:761626 
Change-Id: Ic608ced5dcf662369e4695118c28794ad3873255
Reviewed-on: https://chromium-review.googlesource.com/788430
Reviewed-by: dsinclair <dsinclair@chromium.org>
Reviewed-by: Paul Meyer <paulmeyer@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519722}
[modify] https://crrev.com/0e3efb73180eac56a29a867720d41a3ffa04e032/pdf/pdfium/pdfium_engine.cc
[modify] https://crrev.com/0e3efb73180eac56a29a867720d41a3ffa04e032/pdf/pdfium/pdfium_range.cc

ClusterFuzz has detected this issue as fixed in range 501053:501100.

Detailed report: https://clusterfuzz.com/testcase?key=6041597883187200

Fuzzer: ifratric_acrojs
Job Type: windows_asan_chrome_no_sandbox
Platform Id: windows

Crash Type: Stack-buffer-overflow WRITE {*}
Crash Address: 0x0053c6a8
Crash State:
  FPDFText_GetText
  chrome_pdf::PDFiumRange::GetText
  chrome_pdf::PDFiumEngine::GetSelectedText
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=499331:499354
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=501053:501100

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6041597883187200

Additional requirements: Requires Gestures

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot