New issue
Advanced search Search tips

Issue 831583 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

pdfium: signed-integer-overflow in CPDF_Font::FallbackFontFromCharcode

Reported by pdk...@gmail.com, Apr 11 2018

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.70 Safari/537.36

Steps to reproduce the problem:
I doubt there are security implications here, but I'll let someone else decide on that.

../../core/fpdfapi/font/cpdf_font.cpp:458:43: runtime error: signed integer overflow: 903424266 * 5 cannot be represented in type 'int'
    #0 0x5b4aaf in CPDF_Font::FallbackFontFromCharcode(unsigned int) ../../core/fpdfapi/font/cpdf_font.cpp:458:43
    #1 0x6e1c1a in CPDF_CharPosList::Load(std::__1::vector<unsigned int, std::__1::allocator<unsigned int> > const&, std::__1::vector<float, std::__1::allocator<float> > const&, CPDF_Font*, float) ../../core/fpdfapi/render/cpdf_charposlist.cpp:52:18
    #2 0x727e9b in CPDF_TextRenderer::DrawNormalText(CFX_RenderDevice*, std::__1::vector<unsigned int, std::__1::allocator<unsigned int> > const&, std::__1::vector<float, std::__1::allocator<float> > const&, CPDF_Font*, float, CFX_Matrix const*, unsigned int, CPDF_RenderOptions const*) ../../core/fpdfapi/render/cpdf_textrenderer.cpp:127:15
    #3 0x720138 in CPDF_RenderStatus::ProcessText(CPDF_TextObject*, CFX_Matrix const*, CFX_PathData*) ../../core/fpdfapi/render/cpdf_renderstatus.cpp:1834:10
    #4 0x71f325 in CPDF_RenderStatus::ProcessObjectNoClip(CPDF_PageObject*, CFX_Matrix const*) ../../core/fpdfapi/render/cpdf_renderstatus.cpp:1198:14
    #5 0x71b357 in CPDF_RenderStatus::ContinueSingleObject(CPDF_PageObject*, CFX_Matrix const*, IFX_PauseIndicator*) ../../core/fpdfapi/render/cpdf_renderstatus.cpp:1154:5
    #6 0x719aa5 in CPDF_ProgressiveRenderer::Continue(IFX_PauseIndicator*) ../../core/fpdfapi/render/cpdf_progressiverenderer.cpp:93:30
    #7 0x718cdc in CPDF_ProgressiveRenderer::Start(IFX_PauseIndicator*) ../../core/fpdfapi/render/cpdf_progressiverenderer.cpp:44:3
    #8 0x50766c in (anonymous namespace)::RenderPageImpl(CPDF_PageRenderContext*, CPDF_Page*, CFX_Matrix const&, FX_RECT const&, int, bool, IFSDK_PAUSE_Adapter*) ../../fpdfsdk/fpdfview.cpp:129:26
    #9 0x5063b0 in FPDF_RenderPage_Retail(CPDF_PageRenderContext*, void*, int, int, int, int, int, int, bool, IFSDK_PAUSE_Adapter*) ../../fpdfsdk/fpdfview.cpp:1280:3
    #10 0x506178 in FPDF_RenderPageBitmap ../../fpdfsdk/fpdfview.cpp:1001:3

What is the expected behavior?

What went wrong?
^

Did this work before? No 

Chrome version: 66.0.3359.70  Channel: n/a
OS Version: Ubuntu 14.04
Flash Version:
 

Comment 1 by pdk...@gmail.com, Apr 11 2018

chromium-831583.pdf
228 bytes Download
Labels: Needs-Feedback
I can't reproduce this one (tried stable and canary), does the crash happen just by rendering the file, or is there something else that needs to be done to trigger it?

Comment 3 by tsepez@chromium.org, Apr 11 2018

Owner: dsinclair@chromium.org
Status: Assigned (was: Unconfirmed)
I'd expect you'd need a UBSAN build.
Components: Internals>Plugins>PDF
Labels: Security_Severity-Low
Re #3: Yeah, missed that, was able to reproduce with an UBSAN build.

Doesn't look like a security issue to me, but setting impact low to stay on the safe side, feel free to remove the restriction (or raise the severity) if appropriate.
Cc: npm@chromium.org

Comment 7 by npm@chromium.org, Apr 13 2018

Cc: -npm@chromium.org
Labels: -Restrict-View-SecurityTeam -Needs-Feedback -Security_Severity-Low
Owner: npm@chromium.org
The value is only used for the font weight, so no security issues. The other places where we could overflow by multiplying the stem by 5 had already been found (ClusterFuzz?) and fixed. Thanks for reporting the bug!
Labels: -Type-Bug-Security Type-Bug
Removing bug-security then, thanks!
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 13 2018

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

commit af2ee2cc2e41709df7afc8f49f11ed2e8cf6dedf
Author: Nicolas Pena <npm@chromium.org>
Date: Fri Apr 13 20:17:26 2018

Fix integer overflow in CPDF_Font::FallbackFontFromCharcode

Bug:  chromium:831583 
Change-Id: Idc980ef47cdd942bddc75d9b7fe4a56bdeacdc1a
Reviewed-on: https://pdfium-review.googlesource.com/30670
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

[modify] https://crrev.com/af2ee2cc2e41709df7afc8f49f11ed2e8cf6dedf/core/fpdfapi/font/cpdf_font.cpp

Comment 10 by npm@chromium.org, Apr 13 2018

Status: Fixed (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 13 2018

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

commit 741fa132890291513a192ba7d4e1ff96fd39de72
Author: pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Apr 13 23:05:59 2018

Roll src/third_party/pdfium/ 996c93068..b71d24c1a (8 commits)

https://pdfium.googlesource.com/pdfium.git/+log/996c93068bfc..b71d24c1affe

$ git log 996c93068..b71d24c1a --date=short --no-merges --format='%ad %ae %s'
2018-04-13 thestig Patch lcms to mark data structures as const.
2018-04-13 npm Fix integer overflow in CPDF_Font::FallbackFontFromCharcode
2018-04-13 hnakashima Add test for circular CPDF_Function::Load().
2018-04-13 thestig Add CPDF_ColorState::SetPattern().
2018-04-13 thestig Get rid of CPDF_Color::GetColorSpace().
2018-04-13 dsinclair Add CPDF_Metadata unittests
2018-04-13 dsinclair Hide XML parsing inside CXFA_XMLLocale
2018-04-13 dsinclair Move SharedForm check to CPDF_Metadata class

Created with:
  roll-dep src/third_party/pdfium
BUG= chromium:831583 , chromium:830221 


The AutoRoll server is located here: https://pdfium-roll.skia.org

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=dsinclair@chromium.org

Change-Id: I5cd3fc28ca70459b9d57cf025cd02fba8e5d81cc
Reviewed-on: https://chromium-review.googlesource.com/1013045
Commit-Queue: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#550775}
[modify] https://crrev.com/741fa132890291513a192ba7d4e1ff96fd39de72/DEPS

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/741fa132890291513a192ba7d4e1ff96fd39de72

commit 741fa132890291513a192ba7d4e1ff96fd39de72
Author: pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Apr 13 23:05:59 2018

Roll src/third_party/pdfium/ 996c93068..b71d24c1a (8 commits)

https://pdfium.googlesource.com/pdfium.git/+log/996c93068bfc..b71d24c1affe

$ git log 996c93068..b71d24c1a --date=short --no-merges --format='%ad %ae %s'
2018-04-13 thestig Patch lcms to mark data structures as const.
2018-04-13 npm Fix integer overflow in CPDF_Font::FallbackFontFromCharcode
2018-04-13 hnakashima Add test for circular CPDF_Function::Load().
2018-04-13 thestig Add CPDF_ColorState::SetPattern().
2018-04-13 thestig Get rid of CPDF_Color::GetColorSpace().
2018-04-13 dsinclair Add CPDF_Metadata unittests
2018-04-13 dsinclair Hide XML parsing inside CXFA_XMLLocale
2018-04-13 dsinclair Move SharedForm check to CPDF_Metadata class

Created with:
  roll-dep src/third_party/pdfium
BUG= chromium:831583 , chromium:830221 


The AutoRoll server is located here: https://pdfium-roll.skia.org

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=dsinclair@chromium.org

Change-Id: I5cd3fc28ca70459b9d57cf025cd02fba8e5d81cc
Reviewed-on: https://chromium-review.googlesource.com/1013045
Commit-Queue: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#550775}
[modify] https://crrev.com/741fa132890291513a192ba7d4e1ff96fd39de72/DEPS

Sign in to add a comment