CVE-2015-6781 (C, published 2015-12-06T01:59Z)
Description: value: Integer overflow in the FontData::Bound function in data/font_data.cc in Google sfntly, as used in Google Chrome before 47.0.2526.73, allows remote attackers to cause a denial of service or possibly have unspecified other impact via a crafted offset or length value within font data in an SFNT container.
Severity: HIGH (CVSS v3 base score: )
Weakness: CWE-189 \u2014 Numeric Errors
Weaknesses in this category are related to improper calculation or conversion of numbers.
Repository: https://github.com/googlei18n/sfntly
Files changed:
cpp/src/sfntly/data/font_data.cc
cpp/src/sfntly/data/font_data.h
cpp/src/sfntly/port/logging.h
Commit message: Check for integer overflow in sfntly::FontData::Bound().

Also delete dead code and cleanup some nits.

This is cl/96914065.


Vulnerable code:
#include <limits.h>
#include "sfntly/data/font_data.h"
bool FontData::Bound(int32_t offset, int32_t length) {
  if (offset + length > Size() || offset < 0 || length < 0)
    return false;

  bound_offset_ += offset;
  return true;
}

bool FontData::Bound(int32_t offset) {
if (offset > Size() || offset < 0)
    return false;

  bound_offset_ += offset;
  return true;
            ? GROWABLE_SIZE : data->bound_length_ - offset);
#include <vector>

#include "sfntly/port/type.h"
  // @return true if the bounding range was successful; false otherwise
  virtual bool Bound(int32_t offset, int32_t length);

  // Sets limits on the size of the FontData. This is a offset bound only so if
  // the FontData is writable and growable then there is no limit to that growth
  // from the bounding operation.
  // @param offset the start of the new bounds which must be within the current
  //        size of the FontData
  // @return true if the bounding range was successful; false otherwise
  virtual bool Bound(int32_t offset);

Fixed code:
#include "sfntly/data/font_data.h"

#include <limits>
#include "sfntly/port/logging.h"
void FontData::Bound(int32_t offset, int32_t length) {
  // Inputs should not be negative.
  CHECK(offset >= 0);
  CHECK(length >= 0);

  // Check to make sure |bound_offset_| will not overflow.
  CHECK(bound_offset_ <= std::numeric_limits<int32_t>::max() - offset);
  const int32_t new_offset = bound_offset_ + offset;

  if (length == GROWABLE_SIZE) {
    // When |length| has the special value of GROWABLE_SIZE, it means the size
    // should not have any artificial limits, thus it is just the underlying
    // |array_|'s size. Just make sure |new_offset| is still within bounds.
    CHECK(new_offset <= array_->Size());
  } else {
    // When |length| has any other value, |new_offset| + |length| points to the
    // end of the array. Make sure that is within bounds, but use subtraction to
    // avoid an integer overflow.
    CHECK(new_offset <= array_->Size() - length);
  }

  bound_offset_ = new_offset;
        ? GROWABLE_SIZE : data->bound_length_ - offset);
#include "sfntly/port/type.h"
  virtual void Bound(int32_t offset, int32_t length);
/*
 * Copyright 2015 Google Inc. All Rights Reserved.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

#ifndef SFNTLY_CPP_SRC_SFNTLY_PORT_LOGGING_H_
#define SFNTLY_CPP_SRC_SFNTLY_PORT_LOGGING_H_

#include <stdio.h>
#include <stdlib.h>

// Cheap base/logging.h knock off.

#define CHECK(expr) \
    if (!(expr)) { \
      printf("CHECK failed\n"); \
      abort(); \
    }

#endif  // SFNTLY_CPP_SRC_SFNTLY_PORT_LOGGING_H_