New issue
Advanced search Search tips

Issue 645036 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: Bug



Sign in to add a comment

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]

 
poc.pdf
631 bytes Download
I uploaded a patch for this issue at https://codereview.chromium.org/2323663002

Comment 3 by wfh@chromium.org, Sep 8 2016

Components: Internals>Plugins>PDF
Owner: och...@chromium.org
Status: Assigned (was: Unconfirmed)
ochang, not sure if this is a security bug or not, can you help triage?
Labels: -Type-Bug-Security -Restrict-View-SecurityTeam Type-Bug
Summary: Comparing signed and unsigned values in CPDF_ImageRenderer::StartDIBSource (was: Security: Comparing signed and unsigned values in CPDF_ImageRenderer::StartDIBSource)
Removing security labels. I don't see an obvious way this can be exploited.
Project Member

Comment 5 by bugdroid1@chromium.org, 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

Project Member

Comment 6 by bugdroid1@chromium.org, 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

Cc: och...@chromium.org
Owner: ----
Status: Fixed (was: Assigned)

Sign in to add a comment