Comparing signed and unsigned values in CPDF_ImageRenderer::StartDIBSource
Reported by
stackexp...@gmail.com,
Sep 8 2016
|
||||
Issue description
VULNERABILITY DETAILS
Not sure this should be labeled as Security or not.
A compare was made between signed and unsigned values. I'm not sure whether it has security impact or not.
The issue lies in function CPDF_ImageRenderer::StartDIBSource().
FX_BOOL CPDF_ImageRenderer::StartDIBSource() {
if (!(m_Flags & RENDER_FORCE_DOWNSAMPLE) && m_pDIBSource->GetBPP() > 1) {
int image_size = m_pDIBSource->GetBPP() / 8 * m_pDIBSource->GetWidth() *
m_pDIBSource->GetHeight();
if (image_size > FPDF_HUGE_IMAGE_SIZE &&
!(m_Flags & RENDER_FORCE_HALFTONE)) {
m_Flags |= RENDER_FORCE_DOWNSAMPLE;
}
}
// ......
}
Here image_size is an int variable. The value of it can be negative such that the compare in the if statement can be bypassed. For example, the value of image_size can be 0xffff4740, it will be expressed as -47296 when considered as an int.
We can solve this issue by using type size_t instead of int.
VERSION
Chrome Version: [x.x.x.x] + [stable, beta, or dev]
Operating System: [Please indicate OS, version, and service pack level]
REPRODUCTION CASE
Please include a demonstration of the security bug, such as an attached
HTML or binary file that reproduces the bug when loaded in Chrome. PLEASE
make the file as small as possible and remove any content not required to
demonstrate the bug.
FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: [tab, browser, etc.]
Crash State: [see link above: stack trace, registers, exception record]
Client ID (if relevant): [see link above]
,
Sep 8 2016
I uploaded a patch for this issue at https://codereview.chromium.org/2323663002
,
Sep 8 2016
ochang, not sure if this is a security bug or not, can you help triage?
,
Sep 9 2016
Removing security labels. I don't see an obvious way this can be exploited.
,
Sep 19 2016
The following revision refers to this bug: https://pdfium.googlesource.com/pdfium.git/+/2f8568ef91156d2deb8411c427fbb52f880ccc34 commit 2f8568ef91156d2deb8411c427fbb52f880ccc34 Author: stackexploit <stackexploit@gmail.com> Date: Mon Sep 19 14:05:50 2016 Fix compare between signed and unsigned values in CPDF_ImageRenderer::StartDIBSource. Correct the compare logic in CPDF_ImageRenderer::StartDIBSource() by using size_t instead of int. BUG= chromium:645036 R=ochang@chromium.org Review-Url: https://codereview.chromium.org/2323663002 [modify] https://crrev.com/2f8568ef91156d2deb8411c427fbb52f880ccc34/core/fpdfapi/fpdf_render/fpdf_render_image.cpp
,
Sep 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7c4bbe1f0d4db8059ac7bf7c06f49a45470f4434 commit 7c4bbe1f0d4db8059ac7bf7c06f49a45470f4434 Author: pdfium-deps-roller <pdfium-deps-roller@chromium.org> Date: Mon Sep 19 15:41:15 2016 Roll src/third_party/pdfium/ a0ff010a3..ea3c3be83 (2 commits). https://pdfium.googlesource.com/pdfium.git/+log/a0ff010a380c..ea3c3be83dae $ git log a0ff010a3..ea3c3be83 --date=short --no-merges --format='%ad %ae %s' 2016-09-19 npm Remove duplicated charset definitions, and move them to fx_font.h 2016-09-19 stackexploit Fix compare between signed and unsigned values in CPDF_ImageRenderer::StartDIBSource. BUG= 645036 TBR=dsinclair@chromium.org Review-Url: https://codereview.chromium.org/2349213002 Cr-Commit-Position: refs/heads/master@{#419466} [modify] https://crrev.com/7c4bbe1f0d4db8059ac7bf7c06f49a45470f4434/DEPS
,
Sep 19 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by stackexp...@gmail.com
, Sep 8 2016631 bytes
631 bytes Download