From 067243962cf9663a6e055dfb8b853c7f0b4319af Mon Sep 17 00:00:00 2001 From: stuartg Date: Wed, 16 Oct 2013 23:04:01 +0000 Subject: ticket:38 Fix sfntly compilation on VS2013 --- cpp/src/sfntly/data/writable_font_data.cc | 2 ++ cpp/src/sfntly/port/file_input_stream.cc | 2 ++ cpp/src/sfntly/port/memory_input_stream.cc | 2 ++ cpp/src/sfntly/table/core/os2_table.cc | 2 ++ 4 files changed, 8 insertions(+) (limited to 'cpp') diff --git a/cpp/src/sfntly/data/writable_font_data.cc b/cpp/src/sfntly/data/writable_font_data.cc index 7f6f72f..4b3b440 100644 --- a/cpp/src/sfntly/data/writable_font_data.cc +++ b/cpp/src/sfntly/data/writable_font_data.cc @@ -16,6 +16,8 @@ #include "sfntly/data/writable_font_data.h" +#include + #include "sfntly/data/memory_byte_array.h" #include "sfntly/data/growable_memory_byte_array.h" diff --git a/cpp/src/sfntly/port/file_input_stream.cc b/cpp/src/sfntly/port/file_input_stream.cc index 5bcb434..dfe9a7b 100644 --- a/cpp/src/sfntly/port/file_input_stream.cc +++ b/cpp/src/sfntly/port/file_input_stream.cc @@ -18,6 +18,8 @@ #include #endif +#include + #include "sfntly/port/file_input_stream.h" #include "sfntly/port/exception_type.h" diff --git a/cpp/src/sfntly/port/memory_input_stream.cc b/cpp/src/sfntly/port/memory_input_stream.cc index 56ee81e..f6f2b9b 100755 --- a/cpp/src/sfntly/port/memory_input_stream.cc +++ b/cpp/src/sfntly/port/memory_input_stream.cc @@ -20,6 +20,8 @@ #include +#include + #include "sfntly/port/memory_input_stream.h" #include "sfntly/port/exception_type.h" diff --git a/cpp/src/sfntly/table/core/os2_table.cc b/cpp/src/sfntly/table/core/os2_table.cc index 7ca9d9a..1fef309 100644 --- a/cpp/src/sfntly/table/core/os2_table.cc +++ b/cpp/src/sfntly/table/core/os2_table.cc @@ -16,6 +16,8 @@ #include "sfntly/table/core/os2_table.h" +#include + namespace sfntly { /****************************************************************************** * Constants -- cgit v1.2.3 From f139ed9dd98ad05c7e5cfd7228597fe13b608407 Mon Sep 17 00:00:00 2001 From: arthurhsu Date: Tue, 5 Nov 2013 18:44:58 +0000 Subject: Issue 19330043: fix QNX build Patch by: efidler1@blackberry.com --- cpp/src/sample/chromium/font_subsetter.h | 2 +- cpp/src/sfntly/port/type.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sample/chromium/font_subsetter.h b/cpp/src/sample/chromium/font_subsetter.h index 07b1b5b..b891784 100644 --- a/cpp/src/sample/chromium/font_subsetter.h +++ b/cpp/src/sample/chromium/font_subsetter.h @@ -18,7 +18,7 @@ #ifndef SFNTLY_CPP_SRC_TEST_FONT_SUBSETTER_H_ #define SFNTLY_CPP_SRC_TEST_FONT_SUBSETTER_H_ -#include +#include class SfntlyWrapper { public: diff --git a/cpp/src/sfntly/port/type.h b/cpp/src/sfntly/port/type.h index 20a5ba8..9f82a5a 100644 --- a/cpp/src/sfntly/port/type.h +++ b/cpp/src/sfntly/port/type.h @@ -41,7 +41,7 @@ #include #endif -#include +#include #include #include -- cgit v1.2.3 From 97207cee12e945fd45471488f555c76b17644dec Mon Sep 17 00:00:00 2001 From: arthurhsu Date: Mon, 9 Jun 2014 21:12:10 +0000 Subject: Fix compiler warning Clang warning: 'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false [-Wtautological-undefined-compare] --- cpp/src/sfntly/table/bitmap/bitmap_glyph_info.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/table/bitmap/bitmap_glyph_info.cc b/cpp/src/sfntly/table/bitmap/bitmap_glyph_info.cc index ab9953b..a299b3e 100644 --- a/cpp/src/sfntly/table/bitmap/bitmap_glyph_info.cc +++ b/cpp/src/sfntly/table/bitmap/bitmap_glyph_info.cc @@ -52,7 +52,7 @@ bool BitmapGlyphInfo::operator==(const BitmapGlyphInfo& rhs) const { bool BitmapGlyphInfo::operator==(BitmapGlyphInfo* rhs) { if (rhs == NULL) { - return this == NULL; + return false; // Well defined C++ code's this is always not null. } return (format_ == rhs->format() && glyph_id_ == rhs->glyph_id() && -- cgit v1.2.3 From de776d4ef06ca29c240de3444348894f032b03ff Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 23 Sep 2015 16:06:28 -0700 Subject: Check for integer overflow in sfntly::FontData::Bound(). Also delete dead code and cleanup some nits. This is cl/96914065. --- cpp/src/sfntly/data/font_data.cc | 44 ++++++++++++++++++++++++---------------- cpp/src/sfntly/data/font_data.h | 15 ++------------ cpp/src/sfntly/port/logging.h | 31 ++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 30 deletions(-) create mode 100644 cpp/src/sfntly/port/logging.h (limited to 'cpp') diff --git a/cpp/src/sfntly/data/font_data.cc b/cpp/src/sfntly/data/font_data.cc index d2b95ea..95bee3e 100644 --- a/cpp/src/sfntly/data/font_data.cc +++ b/cpp/src/sfntly/data/font_data.cc @@ -14,11 +14,13 @@ * limitations under the License. */ -#include +#include "sfntly/data/font_data.h" + #include #include +#include -#include "sfntly/data/font_data.h" +#include "sfntly/port/logging.h" namespace sfntly { @@ -26,21 +28,29 @@ int32_t FontData::Size() const { return std::min(array_->Size() - bound_offset_, bound_length_); } -bool FontData::Bound(int32_t offset, int32_t length) { - if (offset + length > Size() || offset < 0 || length < 0) - return false; - - bound_offset_ += offset; +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::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; bound_length_ = length; - return true; -} - -bool FontData::Bound(int32_t offset) { -if (offset > Size() || offset < 0) - return false; - - bound_offset_ += offset; - return true; } int32_t FontData::Length() const { @@ -60,7 +70,7 @@ FontData::FontData(FontData* data, int32_t offset) { Init(data->array_); Bound(data->bound_offset_ + offset, (data->bound_length_ == GROWABLE_SIZE) - ? GROWABLE_SIZE : data->bound_length_ - offset); + ? GROWABLE_SIZE : data->bound_length_ - offset); } FontData::~FontData() {} diff --git a/cpp/src/sfntly/data/font_data.h b/cpp/src/sfntly/data/font_data.h index d02e8b7..e0e7e79 100644 --- a/cpp/src/sfntly/data/font_data.h +++ b/cpp/src/sfntly/data/font_data.h @@ -19,11 +19,9 @@ #include -#include - -#include "sfntly/port/type.h" #include "sfntly/data/byte_array.h" #include "sfntly/port/refcount.h" +#include "sfntly/port/type.h" namespace sfntly { @@ -60,16 +58,7 @@ class FontData : virtual public RefCount { // visible within the bounds set. // @param offset the start of the new bounds // @param length the number of bytes in the bounded array - // @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); + virtual void Bound(int32_t offset, int32_t length); // Makes a slice of this FontData. The returned slice will share the data with // the original FontData. diff --git a/cpp/src/sfntly/port/logging.h b/cpp/src/sfntly/port/logging.h new file mode 100644 index 0000000..1d9e319 --- /dev/null +++ b/cpp/src/sfntly/port/logging.h @@ -0,0 +1,31 @@ +/* + * 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 +#include + +// Cheap base/logging.h knock off. + +#define CHECK(expr) \ + if (!(expr)) { \ + printf("CHECK failed\n"); \ + abort(); \ + } + +#endif // SFNTLY_CPP_SRC_SFNTLY_PORT_LOGGING_H_ -- cgit v1.2.3 From c56b85408bab232efd7e650f0994272a174e3b92 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 9 Jun 2016 23:30:26 -0700 Subject: Add a bounds check to ByteArray::Get(). --- cpp/src/sfntly/data/byte_array.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'cpp') diff --git a/cpp/src/sfntly/data/byte_array.cc b/cpp/src/sfntly/data/byte_array.cc index 915a40c..57f9eed 100644 --- a/cpp/src/sfntly/data/byte_array.cc +++ b/cpp/src/sfntly/data/byte_array.cc @@ -35,6 +35,8 @@ int32_t ByteArray::SetFilledLength(int32_t filled_length) { } int32_t ByteArray::Get(int32_t index) { + if (index < 0 || index >= Length()) + return -1; return InternalGet(index) & 0xff; } -- cgit v1.2.3 From dc29ad099debf894cd9215de7cafcda30731d0f3 Mon Sep 17 00:00:00 2001 From: Hal Canary Date: Tue, 26 Jul 2016 20:36:30 -0400 Subject: SfntlyWrapper::SubsetFont takes TTC index --- cpp/src/sample/chromium/font_subsetter.cc | 21 +++++++++++++++++++++ cpp/src/sample/chromium/font_subsetter.h | 24 ++++++++++++++++++++++++ cpp/src/sample/chromium/subsetter_impl.cc | 18 ++++++++++++++++++ cpp/src/sample/chromium/subsetter_impl.h | 3 +++ 4 files changed, 66 insertions(+) (limited to 'cpp') diff --git a/cpp/src/sample/chromium/font_subsetter.cc b/cpp/src/sample/chromium/font_subsetter.cc index 14f5494..0f9bc13 100644 --- a/cpp/src/sample/chromium/font_subsetter.cc +++ b/cpp/src/sample/chromium/font_subsetter.cc @@ -37,3 +37,24 @@ int SfntlyWrapper::SubsetFont(const char* font_name, return subsetter.SubsetFont(glyph_ids, glyph_count, output_buffer); } + +int SfntlyWrapper::SubsetFont(int font_index, + const unsigned char* original_font, + size_t font_size, + const unsigned int* glyph_ids, + size_t glyph_count, + unsigned char** output_buffer) { + if (output_buffer == NULL || + original_font == NULL || font_size == 0 || + glyph_ids == NULL || glyph_count == 0) { + return 0; + } + + sfntly::SubsetterImpl subsetter; + if (!subsetter.LoadFont(font_index, original_font, font_size)) { + return -1; // Load error or font not found. + } + + return subsetter.SubsetFont(glyph_ids, glyph_count, output_buffer); +} + diff --git a/cpp/src/sample/chromium/font_subsetter.h b/cpp/src/sample/chromium/font_subsetter.h index b891784..c8e65e2 100644 --- a/cpp/src/sample/chromium/font_subsetter.h +++ b/cpp/src/sample/chromium/font_subsetter.h @@ -46,6 +46,30 @@ class SfntlyWrapper { const unsigned int* glyph_ids, size_t glyph_count, unsigned char** output_buffer); + + + // Font subsetting API + // + // Input TTF/TTC/OTF fonts, specify the glyph IDs to subset, and the subset + // font is returned in |output_buffer| (caller to delete[]). Return value is + // the length of output_buffer allocated. + // + // If subsetting fails, a negative value is returned. If none of the glyph + // IDs specified is found, the function will return 0. + // + // |font_name| Font index, ignored for non-TTC files, 0-indexed. + // |original_font| Original font file contents. + // |font_size| Size of |original_font| in bytes. + // |glyph_ids| Glyph IDs to subset. If the specified glyph ID is not + // found in the font file, it will be ignored silently. + // |glyph_count| Number of glyph IDs in |glyph_ids| + // |output_buffer| Generated subset font. Caller to delete[]. + static int SubsetFont(int font_index, + const unsigned char* original_font, + size_t font_size, + const unsigned int* glyph_ids, + size_t glyph_count, + unsigned char** output_buffer); }; #endif // SFNTLY_CPP_SRC_TEST_FONT_SUBSETTER_H_ diff --git a/cpp/src/sample/chromium/subsetter_impl.cc b/cpp/src/sample/chromium/subsetter_impl.cc index 528e336..c53e607 100644 --- a/cpp/src/sample/chromium/subsetter_impl.cc +++ b/cpp/src/sample/chromium/subsetter_impl.cc @@ -616,6 +616,24 @@ SubsetterImpl::SubsetterImpl() { SubsetterImpl::~SubsetterImpl() { } +bool SubsetterImpl::LoadFont(int font_index, + const unsigned char* original_font, + size_t font_size) { + MemoryInputStream mis; + mis.Attach(original_font, font_size); + if (factory_ == NULL) { + factory_.Attach(FontFactory::GetInstance()); + } + + FontArray font_array; + factory_->LoadFonts(&mis, &font_array); + if (font_index < 0 || (size_t)font_index >= font_array.size()) { + return false; + } + font_ = font_array[font_index].p_; + return font_ != NULL; +} + bool SubsetterImpl::LoadFont(const char* font_name, const unsigned char* original_font, size_t font_size) { diff --git a/cpp/src/sample/chromium/subsetter_impl.h b/cpp/src/sample/chromium/subsetter_impl.h index ffbf408..738a8d4 100644 --- a/cpp/src/sample/chromium/subsetter_impl.h +++ b/cpp/src/sample/chromium/subsetter_impl.h @@ -57,6 +57,9 @@ class SubsetterImpl { bool LoadFont(const char* font_name, const unsigned char* original_font, size_t font_size); + bool LoadFont(int font_index, + const unsigned char* original_font, + size_t font_size); int SubsetFont(const unsigned int* glyph_ids, size_t glyph_count, unsigned char** output_buffer); -- cgit v1.2.3 From 08652be8695a5e3732fa5c3d5d9c0a6d98b3b9e0 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 1 Sep 2016 00:13:10 -0700 Subject: Add a nullptr check to GlyphTable::Glyph(). Do not attempt to create a new Glyph if there is no data. Fixes https://crbug.com/641330 --- cpp/src/sfntly/table/truetype/glyph_table.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/table/truetype/glyph_table.cc b/cpp/src/sfntly/table/truetype/glyph_table.cc index f38fac5..0c1e841 100644 --- a/cpp/src/sfntly/table/truetype/glyph_table.cc +++ b/cpp/src/sfntly/table/truetype/glyph_table.cc @@ -215,10 +215,11 @@ CALLER_ATTACH GlyphTable::Glyph* ReadableFontDataPtr sliced_data; sliced_data.Attach(down_cast(data->Slice(offset, length))); - if (type == GlyphType::kSimple) { - glyph = new SimpleGlyph(sliced_data); - } else { - glyph = new CompositeGlyph(sliced_data); + if (sliced_data) { + if (type == GlyphType::kSimple) + glyph = new SimpleGlyph(sliced_data); + else + glyph = new CompositeGlyph(sliced_data); } return glyph.Detach(); } -- cgit v1.2.3 From 813efeb18bf6205354b01a525352ebdb10eebe06 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 1 Sep 2016 00:15:48 -0700 Subject: Add a size limit for font tables. Add a generous 200 MB limit for font tables. Enforce the limit in Font::Builder::LoadTableData(). Clean up some nits along the way. Fixes https://crbug.com/641446 --- cpp/src/sfntly/font.cc | 62 +++++++++++++++++++++++++++----------------------- cpp/src/sfntly/font.h | 2 +- 2 files changed, 35 insertions(+), 29 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/font.cc b/cpp/src/sfntly/font.cc index 347e0c1..cfb956f 100644 --- a/cpp/src/sfntly/font.cc +++ b/cpp/src/sfntly/font.cc @@ -40,24 +40,27 @@ namespace sfntly { -const int32_t SFNTVERSION_MAJOR = 1; -const int32_t SFNTVERSION_MINOR = 0; +namespace { + +const int32_t kSFNTVersionMajor = 1; +const int32_t kSFNTVersionMinor = 0; + +const int32_t kMaxTableSize = 200 * 1024 * 1024; + +} // namespace /****************************************************************************** * Font class ******************************************************************************/ Font::~Font() {} -bool Font::HasTable(int32_t tag) { - TableMap::const_iterator result = tables_.find(tag); - TableMap::const_iterator end = tables_.end(); - return (result != end); +bool Font::HasTable(int32_t tag) const { + return tables_.find(tag) != tables_.end(); } Table* Font::GetTable(int32_t tag) { - if (!HasTable(tag)) { + if (!HasTable(tag)) return NULL; - } return tables_[tag]; } @@ -308,15 +311,12 @@ Table::Builder* Font::Builder::NewTableBuilder(int32_t tag, } void Font::Builder::RemoveTableBuilder(int32_t tag) { - TableBuilderMap::iterator target = table_builders_.find(tag); - if (target != table_builders_.end()) { - table_builders_.erase(target); - } + table_builders_.erase(tag); } Font::Builder::Builder(FontFactory* factory) : factory_(factory), - sfnt_version_(Fixed1616::Fixed(SFNTVERSION_MAJOR, SFNTVERSION_MINOR)) { + sfnt_version_(Fixed1616::Fixed(kSFNTVersionMajor, kSFNTVersionMinor)) { } void Font::Builder::LoadFont(InputStream* is) { @@ -525,32 +525,38 @@ void Font::Builder::LoadTableData(HeaderOffsetSortedSet* headers, FontInputStream* is, DataBlockMap* table_data) { assert(table_data); - for (HeaderOffsetSortedSet::iterator table_header = headers->begin(), + for (HeaderOffsetSortedSet::iterator it = headers->begin(), table_end = headers->end(); - table_header != table_end; - ++table_header) { - is->Skip((*table_header)->offset() - is->position()); - FontInputStream table_is(is, (*table_header)->length()); + it != table_end; + ++it) { + const Ptr
header = *it; + is->Skip(header->offset() - is->position()); + if (header->length() > kMaxTableSize) + continue; + + FontInputStream table_is(is, header->length()); WritableFontDataPtr data; - data.Attach( - WritableFontData::CreateWritableFontData((*table_header)->length())); - data->CopyFrom(&table_is, (*table_header)->length()); - table_data->insert(DataBlockEntry(*table_header, data)); + data.Attach(WritableFontData::CreateWritableFontData(header->length())); + data->CopyFrom(&table_is, header->length()); + table_data->insert(DataBlockEntry(header, data)); } } void Font::Builder::LoadTableData(HeaderOffsetSortedSet* headers, WritableFontData* fd, DataBlockMap* table_data) { - for (HeaderOffsetSortedSet::iterator table_header = headers->begin(), + for (HeaderOffsetSortedSet::iterator it = headers->begin(), table_end = headers->end(); - table_header != table_end; - ++table_header) { + it != table_end; + ++it) { + const Ptr
header = *it; + if (header->length() > kMaxTableSize) + continue; + FontDataPtr sliced_data; - sliced_data.Attach( - fd->Slice((*table_header)->offset(), (*table_header)->length())); + sliced_data.Attach(fd->Slice(header->offset(), header->length())); WritableFontDataPtr data = down_cast(sliced_data.p_); - table_data->insert(DataBlockEntry(*table_header, data)); + table_data->insert(DataBlockEntry(header, data)); } } diff --git a/cpp/src/sfntly/font.h b/cpp/src/sfntly/font.h index 975e8cc..2220adb 100644 --- a/cpp/src/sfntly/font.h +++ b/cpp/src/sfntly/font.h @@ -245,7 +245,7 @@ class Font : public RefCounted { int32_t num_tables() { return (int32_t)tables_.size(); } // Whether the font has a particular table. - bool HasTable(int32_t tag); + bool HasTable(int32_t tag) const; // UNIMPLEMENTED: public Iterator iterator -- cgit v1.2.3 From dd230461d199c49d762185d0b02107505d216bee Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 1 Sep 2016 00:18:38 -0700 Subject: Return error values in ReadableFontData::Read*(). Change Read*Byte methods in ReadableFontData to return sentinel values on error. Check the return values in other methods, and repeat the process if they are also Read() methods. Fixes https://crbug.com/641460 --- cpp/src/sfntly/data/readable_font_data.cc | 98 +++++++++++++++++++++---------- cpp/src/sfntly/data/readable_font_data.h | 15 +++-- 2 files changed, 77 insertions(+), 36 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/data/readable_font_data.cc b/cpp/src/sfntly/data/readable_font_data.cc index 06d783f..0f93fdb 100644 --- a/cpp/src/sfntly/data/readable_font_data.cc +++ b/cpp/src/sfntly/data/readable_font_data.cc @@ -59,23 +59,25 @@ void ReadableFontData::SetCheckSumRanges(const IntegerList& ranges) { int32_t ReadableFontData::ReadUByte(int32_t index) { int32_t b = array_->Get(BoundOffset(index)); -#if !defined (SFNTLY_NO_EXCEPTION) if (b < 0) { +#if !defined (SFNTLY_NO_EXCEPTION) throw IndexOutOfBoundException( "Index attempted to be read from is out of bounds", index); - } #endif + return -1; + } return b; } int32_t ReadableFontData::ReadByte(int32_t index) { int32_t b = array_->Get(BoundOffset(index)); -#if !defined (SFNTLY_NO_EXCEPTION) if (b < 0) { +#if !defined (SFNTLY_NO_EXCEPTION) throw IndexOutOfBoundException( "Index attempted to be read from is out of bounds", index); - } #endif + return kInvalidByte; + } return (b << 24) >> 24; } @@ -91,24 +93,52 @@ int32_t ReadableFontData::ReadChar(int32_t index) { } int32_t ReadableFontData::ReadUShort(int32_t index) { - return 0xffff & (ReadUByte(index) << 8 | ReadUByte(index + 1)); + int32_t b1 = ReadUByte(index); + if (b1 < 0) + return -1; + int32_t b2 = ReadUByte(index + 1); + if (b2 < 0) + return -1; + return 0xffff & (b1 << 8 | b2); } int32_t ReadableFontData::ReadShort(int32_t index) { - return ((ReadByte(index) << 8 | ReadUByte(index + 1)) << 16) >> 16; + int32_t b1 = ReadByte(index); + if (b1 == kInvalidByte) + return kInvalidShort; + int32_t b2 = ReadUByte(index + 1); + if (b2 < 0) + return kInvalidShort; + return ((b1 << 8 | b2) << 16) >> 16; } int32_t ReadableFontData::ReadUInt24(int32_t index) { - return 0xffffff & (ReadUByte(index) << 16 | - ReadUByte(index + 1) << 8 | - ReadUByte(index + 2)); + int32_t b1 = ReadUByte(index); + if (b1 < 0) + return -1; + int32_t b2 = ReadUByte(index + 1); + if (b2 < 0) + return -1; + int32_t b3 = ReadUByte(index + 2); + if (b3 < 0) + return -1; + return 0xffffff & (b1 << 16 | b2 << 8 | b3); } int64_t ReadableFontData::ReadULong(int32_t index) { - return 0xffffffffL & (ReadUByte(index) << 24 | - ReadUByte(index + 1) << 16 | - ReadUByte(index + 2) << 8 | - ReadUByte(index + 3)); + int32_t b1 = ReadUByte(index); + if (b1 < 0) + return -1; + int32_t b2 = ReadUByte(index + 1); + if (b2 < 0) + return -1; + int32_t b3 = ReadUByte(index + 2); + if (b3 < 0) + return -1; + int32_t b4 = ReadUByte(index + 3); + if (b4 < 0) + return -1; + return 0xffffffffL & (b1 << 24 | b2 << 16 | b3 << 8 | b4); } int32_t ReadableFontData::ReadULongAsInt(int32_t index) { @@ -122,10 +152,19 @@ int32_t ReadableFontData::ReadULongAsInt(int32_t index) { } int64_t ReadableFontData::ReadULongLE(int32_t index) { - return 0xffffffffL & (ReadUByte(index) | - ReadUByte(index + 1) << 8 | - ReadUByte(index + 2) << 16 | - ReadUByte(index + 3) << 24); + int32_t b1 = ReadUByte(index); + if (b1 < 0) + return -1; + int32_t b2 = ReadUByte(index + 1); + if (b2 < 0) + return -1; + int32_t b3 = ReadUByte(index + 2); + if (b3 < 0) + return -1; + int32_t b4 = ReadUByte(index + 3); + if (b4 < 0) + return -1; + return 0xffffffffL & (b1 | b2 << 8 | b3 << 16 | b4 << 24); } int32_t ReadableFontData::ReadLong(int32_t index) { @@ -187,12 +226,11 @@ int32_t ReadableFontData::SearchUShort(int32_t start_index, #if defined (SFNTLY_DEBUG_FONTDATA) fprintf(stderr, "**start: %d; end: %d\n", location_start, location_end); #endif - if (key <= location_end) { + if (key <= location_end) return location; - } else { - // location is above the current location - bottom = location + 1; - } + + // location is above the current location + bottom = location + 1; } } return -1; @@ -208,14 +246,15 @@ int32_t ReadableFontData::SearchUShort(int32_t start_index, while (top != bottom) { location = (top + bottom) / 2; int32_t location_start = ReadUShort(start_index + location * start_offset); + if (key == location_start) + return location; + if (key < location_start) { // location is below current location top = location; - } else if (key > location_start) { + } else { // location is above current location bottom = location + 1; - } else { - return location; } } return -1; @@ -243,12 +282,11 @@ int32_t ReadableFontData::SearchULong(int32_t start_index, #if defined (SFNTLY_DEBUG_FONTDATA) fprintf(stderr, "**start: %d; end: %d\n", location_start, location_end); #endif - if (key <= location_end) { + if (key <= location_end) return location; - } else { - // location is above the current location - bottom = location + 1; - } + + // location is above the current location + bottom = location + 1; } } return -1; diff --git a/cpp/src/sfntly/data/readable_font_data.h b/cpp/src/sfntly/data/readable_font_data.h index b43c626..a4a3e27 100644 --- a/cpp/src/sfntly/data/readable_font_data.h +++ b/cpp/src/sfntly/data/readable_font_data.h @@ -22,8 +22,8 @@ namespace sfntly { -class WritableFontData; class OutputStream; +class WritableFontData; // Writable font data wrapper. Supports reading of data primitives in the // TrueType / OpenType spec. @@ -51,6 +51,9 @@ class ReadableFontData : public FontData, explicit ReadableFontData(ByteArray* array); virtual ~ReadableFontData(); + static const int32_t kInvalidByte = 128; + static const int32_t kInvalidShort = 32768; + static CALLER_ATTACH ReadableFontData* CreateReadableFontData(ByteVector* b); // Gets a computed checksum for the data. This checksum uses the OpenType spec @@ -76,7 +79,7 @@ class ReadableFontData : public FontData, // Read the BYTE at the given index. // @param index index into the font data - // @return the BYTE + // @return the BYTE; |kInvalidByte| if outside the bounds of the font data // @throws IndexOutOfBoundsException if index is outside the FontData's range virtual int32_t ReadByte(int32_t index); @@ -94,25 +97,25 @@ class ReadableFontData : public FontData, // Read the CHAR at the given index. // @param index index into the font data - // @return the CHAR + // @return the CHAR; -1 if outside the bounds of the font data // @throws IndexOutOfBoundsException if index is outside the FontData's range virtual int32_t ReadChar(int32_t index); // Read the USHORT at the given index. // @param index index into the font data - // @return the USHORT + // @return the USHORT; -1 if outside the bounds of the font data // @throws IndexOutOfBoundsException if index is outside the FontData's range virtual int32_t ReadUShort(int32_t index); // Read the SHORT at the given index. // @param index index into the font data - // @return the SHORT + // @return the SHORT; |kInvalidShort| if outside the bounds of the font data // @throws IndexOutOfBoundsException if index is outside the FontData's range virtual int32_t ReadShort(int32_t index); // Read the UINT24 at the given index. // @param index index into the font data - // @return the UINT24 + // @return the UINT24; -1 if outside the bounds of the font data // @throws IndexOutOfBoundsException if index is outside the FontData's range virtual int32_t ReadUInt24(int32_t index); -- cgit v1.2.3 From c9025eccdd389c91a7ff273976de794317c6928e Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 1 Sep 2016 00:23:47 -0700 Subject: Add more bounds checks in WritableFontData. WritableFontData::Slice() needs to do more input validation. Same for ReadableFontData::Slice(). Same for the equivalent Java code. Fixes https://crbug.com/642300 --- cpp/src/sfntly/data/readable_font_data.cc | 4 +++- cpp/src/sfntly/data/writable_font_data.cc | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/data/readable_font_data.cc b/cpp/src/sfntly/data/readable_font_data.cc index 0f93fdb..9ffcb00 100644 --- a/cpp/src/sfntly/data/readable_font_data.cc +++ b/cpp/src/sfntly/data/readable_font_data.cc @@ -294,7 +294,9 @@ int32_t ReadableFontData::SearchULong(int32_t start_index, CALLER_ATTACH FontData* ReadableFontData::Slice(int32_t offset, int32_t length) { - if (offset < 0 || offset + length > Size()) { + if (offset < 0 || length < 0 || + offset > std::numeric_limits::max() - length || + offset + length > Size()) { #if !defined (SFNTLY_NO_EXCEPTION) throw IndexOutOfBoundsException( "Attempt to bind data outside of its limits"); diff --git a/cpp/src/sfntly/data/writable_font_data.cc b/cpp/src/sfntly/data/writable_font_data.cc index 4b3b440..073e9df 100644 --- a/cpp/src/sfntly/data/writable_font_data.cc +++ b/cpp/src/sfntly/data/writable_font_data.cc @@ -17,6 +17,7 @@ #include "sfntly/data/writable_font_data.h" #include +#include #include "sfntly/data/memory_byte_array.h" #include "sfntly/data/growable_memory_byte_array.h" @@ -167,7 +168,9 @@ void WritableFontData::CopyFrom(InputStream* is) { CALLER_ATTACH FontData* WritableFontData::Slice(int32_t offset, int32_t length) { - if (offset < 0 || offset + length > Size()) { + if (offset < 0 || length < 0 || + offset > std::numeric_limits::max() - length || + offset + length > Size()) { #if !defined (SFNTLY_NO_EXCEPTION) throw IndexOutOfBoundsException( "Attempt to bind data outside of its limits"); @@ -179,7 +182,7 @@ CALLER_ATTACH FontData* WritableFontData::Slice(int32_t offset, } CALLER_ATTACH FontData* WritableFontData::Slice(int32_t offset) { - if (offset > Size()) { + if (offset < 0 || offset > Size()) { #if !defined (SFNTLY_NO_EXCEPTION) throw IndexOutOfBoundsException( "Attempt to bind data outside of its limits"); -- cgit v1.2.3 From 4f1aa4907e2924d2bf4a3794b42e47c7a327c232 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 7 Sep 2016 01:32:08 -0700 Subject: Add ReadableFontData::kInvalidUnsigned. Use in place of -1 where applicable. --- cpp/src/sfntly/data/readable_font_data.cc | 28 ++++++++++++++-------------- cpp/src/sfntly/data/readable_font_data.h | 1 + 2 files changed, 15 insertions(+), 14 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/data/readable_font_data.cc b/cpp/src/sfntly/data/readable_font_data.cc index 9ffcb00..1784693 100644 --- a/cpp/src/sfntly/data/readable_font_data.cc +++ b/cpp/src/sfntly/data/readable_font_data.cc @@ -64,7 +64,7 @@ int32_t ReadableFontData::ReadUByte(int32_t index) { throw IndexOutOfBoundException( "Index attempted to be read from is out of bounds", index); #endif - return -1; + return kInvalidUnsigned; } return b; } @@ -95,10 +95,10 @@ int32_t ReadableFontData::ReadChar(int32_t index) { int32_t ReadableFontData::ReadUShort(int32_t index) { int32_t b1 = ReadUByte(index); if (b1 < 0) - return -1; + return kInvalidUnsigned; int32_t b2 = ReadUByte(index + 1); if (b2 < 0) - return -1; + return kInvalidUnsigned; return 0xffff & (b1 << 8 | b2); } @@ -115,29 +115,29 @@ int32_t ReadableFontData::ReadShort(int32_t index) { int32_t ReadableFontData::ReadUInt24(int32_t index) { int32_t b1 = ReadUByte(index); if (b1 < 0) - return -1; + return kInvalidUnsigned; int32_t b2 = ReadUByte(index + 1); if (b2 < 0) - return -1; + return kInvalidUnsigned; int32_t b3 = ReadUByte(index + 2); if (b3 < 0) - return -1; + return kInvalidUnsigned; return 0xffffff & (b1 << 16 | b2 << 8 | b3); } int64_t ReadableFontData::ReadULong(int32_t index) { int32_t b1 = ReadUByte(index); if (b1 < 0) - return -1; + return kInvalidUnsigned; int32_t b2 = ReadUByte(index + 1); if (b2 < 0) - return -1; + return kInvalidUnsigned; int32_t b3 = ReadUByte(index + 2); if (b3 < 0) - return -1; + return kInvalidUnsigned; int32_t b4 = ReadUByte(index + 3); if (b4 < 0) - return -1; + return kInvalidUnsigned; return 0xffffffffL & (b1 << 24 | b2 << 16 | b3 << 8 | b4); } @@ -154,16 +154,16 @@ int32_t ReadableFontData::ReadULongAsInt(int32_t index) { int64_t ReadableFontData::ReadULongLE(int32_t index) { int32_t b1 = ReadUByte(index); if (b1 < 0) - return -1; + return kInvalidUnsigned; int32_t b2 = ReadUByte(index + 1); if (b2 < 0) - return -1; + return kInvalidUnsigned; int32_t b3 = ReadUByte(index + 2); if (b3 < 0) - return -1; + return kInvalidUnsigned; int32_t b4 = ReadUByte(index + 3); if (b4 < 0) - return -1; + return kInvalidUnsigned; return 0xffffffffL & (b1 | b2 << 8 | b3 << 16 | b4 << 24); } diff --git a/cpp/src/sfntly/data/readable_font_data.h b/cpp/src/sfntly/data/readable_font_data.h index a4a3e27..003d494 100644 --- a/cpp/src/sfntly/data/readable_font_data.h +++ b/cpp/src/sfntly/data/readable_font_data.h @@ -53,6 +53,7 @@ class ReadableFontData : public FontData, static const int32_t kInvalidByte = 128; static const int32_t kInvalidShort = 32768; + static const int32_t kInvalidUnsigned = -1; static CALLER_ATTACH ReadableFontData* CreateReadableFontData(ByteVector* b); -- cgit v1.2.3 From 1ef790afdd7818c1a3c76b18daacff14fdb983aa Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 7 Sep 2016 21:32:59 -0700 Subject: Add missing header from commit c9025ecc. --- cpp/src/sfntly/data/readable_font_data.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'cpp') diff --git a/cpp/src/sfntly/data/readable_font_data.cc b/cpp/src/sfntly/data/readable_font_data.cc index 1784693..df33f9a 100644 --- a/cpp/src/sfntly/data/readable_font_data.cc +++ b/cpp/src/sfntly/data/readable_font_data.cc @@ -18,6 +18,8 @@ #include +#include + #include "sfntly/data/memory_byte_array.h" #include "sfntly/data/writable_font_data.h" #include "sfntly/port/exception_type.h" -- cgit v1.2.3 From 6d1efaa8a89ea563247747e99faf0c44e35842d3 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 19 Oct 2016 14:44:47 -0700 Subject: Fix out of bound access in subtly sample program. Fixes https://crbug.com/638573 --- cpp/src/sample/subtly/font_assembler.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'cpp') diff --git a/cpp/src/sample/subtly/font_assembler.cc b/cpp/src/sample/subtly/font_assembler.cc index 2f7cd11..4717512 100644 --- a/cpp/src/sample/subtly/font_assembler.cc +++ b/cpp/src/sample/subtly/font_assembler.cc @@ -211,6 +211,8 @@ bool FontAssembler::AssembleGlyphAndLocaTables() { // If there are missing glyphs between the last glyph_id and the // current resolved_glyph_id, since the LOCA table needs to have the same // size, the offset is kept the same. + loca_list.resize(std::max(loca_list.size(), + static_cast(resolved_glyph_id + 2))); for (int32_t i = last_glyph_id + 1; i <= resolved_glyph_id; ++i) loca_list[i] = last_offset; last_offset += length; -- cgit v1.2.3 From 083b02b10572142d9863d945a8cf52fed2df997d Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 19 Oct 2016 14:45:57 -0700 Subject: Fix NULL pointer derefs in sfntly::Font::Builder. Fixes https://crbug.com/641452 --- cpp/src/sfntly/font.cc | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/font.cc b/cpp/src/sfntly/font.cc index cfb956f..0c268cc 100644 --- a/cpp/src/sfntly/font.cc +++ b/cpp/src/sfntly/font.cc @@ -393,14 +393,18 @@ void Font::Builder::BuildTablesFromBuilders(Font* font, } static Table::Builder* GetBuilder(TableBuilderMap* builder_map, int32_t tag) { - if (builder_map) { - TableBuilderMap::iterator target = builder_map->find(tag); - if (target != builder_map->end()) { - return target->second.p_; - } - } + if (!builder_map) + return NULL; - return NULL; + TableBuilderMap::iterator target = builder_map->find(tag); + if (target == builder_map->end()) + return NULL; + + Table::Builder* builder = target->second.p_; + if (!builder || !builder->InternalReadData()) + return NULL; + + return builder; } void Font::Builder::InterRelateBuilders(TableBuilderMap* builder_map) { -- cgit v1.2.3 From 1fba3b37c98301e2c01a5a3a7a87693ecdf4b4c8 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 19 Oct 2016 14:47:17 -0700 Subject: Fix undefined shifts in ReadableFontData::ReadLong. Fixes https://crbug.com/646300 --- cpp/src/sfntly/data/readable_font_data.cc | 25 ++++++++++++++++++++----- cpp/src/sfntly/data/readable_font_data.h | 9 ++++++--- 2 files changed, 26 insertions(+), 8 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/data/readable_font_data.cc b/cpp/src/sfntly/data/readable_font_data.cc index df33f9a..93678ff 100644 --- a/cpp/src/sfntly/data/readable_font_data.cc +++ b/cpp/src/sfntly/data/readable_font_data.cc @@ -170,10 +170,19 @@ int64_t ReadableFontData::ReadULongLE(int32_t index) { } int32_t ReadableFontData::ReadLong(int32_t index) { - return ReadByte(index) << 24 | - ReadUByte(index + 1) << 16 | - ReadUByte(index + 2) << 8 | - ReadUByte(index + 3); + int32_t b1 = ReadByte(index); + if (b1 == kInvalidByte) + return kInvalidLong; + int32_t b2 = ReadUByte(index + 1); + if (b2 < 0) + return kInvalidLong; + int32_t b3 = ReadUByte(index + 2); + if (b3 < 0) + return kInvalidLong; + int32_t b4 = ReadUByte(index + 3); + if (b4 < 0) + return kInvalidLong; + return static_cast(b1) << 24 | b2 << 16 | b3 << 8 | b4; } int32_t ReadableFontData::ReadFixed(int32_t index) { @@ -181,7 +190,13 @@ int32_t ReadableFontData::ReadFixed(int32_t index) { } int64_t ReadableFontData::ReadDateTimeAsLong(int32_t index) { - return (int64_t)ReadULong(index) << 32 | ReadULong(index + 4); + int32_t high = ReadULong(index); + if (high == kInvalidUnsigned) + return kInvalidLongDateTime; + int32_t low = ReadULong(index + 4); + if (low == kInvalidUnsigned) + return kInvalidLongDateTime; + return (int64_t)high << 32 | low; } int32_t ReadableFontData::ReadFWord(int32_t index) { diff --git a/cpp/src/sfntly/data/readable_font_data.h b/cpp/src/sfntly/data/readable_font_data.h index 003d494..37a0918 100644 --- a/cpp/src/sfntly/data/readable_font_data.h +++ b/cpp/src/sfntly/data/readable_font_data.h @@ -53,7 +53,9 @@ class ReadableFontData : public FontData, static const int32_t kInvalidByte = 128; static const int32_t kInvalidShort = 32768; + static const int32_t kInvalidLong = 0xffffffff; static const int32_t kInvalidUnsigned = -1; + static const int64_t kInvalidLongDateTime = -1; static CALLER_ATTACH ReadableFontData* CreateReadableFontData(ByteVector* b); @@ -122,7 +124,7 @@ class ReadableFontData : public FontData, // Read the ULONG at the given index. // @param index index into the font data - // @return the ULONG + // @return the ULONG; kInvalidUnsigned if outside the bounds of the font data // @throws IndexOutOfBoundsException if index is outside the FontData's range virtual int64_t ReadULong(int32_t index); @@ -140,7 +142,7 @@ class ReadableFontData : public FontData, // Read the LONG at the given index. // @param index index into the font data - // @return the LONG + // @return the LONG; kInvalidLong if outside the bounds of the font data // @throws IndexOutOfBoundsException if index is outside the FontData's range virtual int32_t ReadLong(int32_t index); @@ -152,7 +154,8 @@ class ReadableFontData : public FontData, // Read the LONGDATETIME at the given index. // @param index index into the font data - // @return the LONGDATETIME + // @return the LONGDATETIME; kInvalidLongDateTime if outside the bounds of the + // font data // @throws IndexOutOfBoundsException if index is outside the FontData's range virtual int64_t ReadDateTimeAsLong(int32_t index); -- cgit v1.2.3 From 8475d2fd5f1ee4c734ea900c88283365a2f0dc87 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 19 Oct 2016 14:49:00 -0700 Subject: Avoid NULL derefs inside FontHeaderTable::Builder. As a result IndexToLocFormat() should also return an invalid value. Fixes https://crbug.com/646347 --- cpp/src/sfntly/table/core/font_header_table.cc | 5 ++++- cpp/src/sfntly/table/core/font_header_table.h | 1 + cpp/src/sfntly/table/table_based_table_builder.cc | 6 ++++-- 3 files changed, 9 insertions(+), 3 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/table/core/font_header_table.cc b/cpp/src/sfntly/table/core/font_header_table.cc index 60015ca..a848afd 100644 --- a/cpp/src/sfntly/table/core/font_header_table.cc +++ b/cpp/src/sfntly/table/core/font_header_table.cc @@ -239,7 +239,10 @@ void FontHeaderTable::Builder::SetFontDirectionHint(int32_t hint) { } int32_t FontHeaderTable::Builder::IndexToLocFormat() { - return down_cast(GetTable())->IndexToLocFormat(); + Table* table = GetTable(); + if (!table) + return IndexToLocFormat::kInvalidOffset; + return down_cast(table)->IndexToLocFormat(); } void FontHeaderTable::Builder::SetIndexToLocFormat(int32_t format) { diff --git a/cpp/src/sfntly/table/core/font_header_table.h b/cpp/src/sfntly/table/core/font_header_table.h index 841955b..4851775 100644 --- a/cpp/src/sfntly/table/core/font_header_table.h +++ b/cpp/src/sfntly/table/core/font_header_table.h @@ -24,6 +24,7 @@ namespace sfntly { struct IndexToLocFormat { enum { + kInvalidOffset = -1, kShortOffset = 0, kLongOffset = 1 }; diff --git a/cpp/src/sfntly/table/table_based_table_builder.cc b/cpp/src/sfntly/table/table_based_table_builder.cc index b505704..51a5a3b 100644 --- a/cpp/src/sfntly/table/table_based_table_builder.cc +++ b/cpp/src/sfntly/table/table_based_table_builder.cc @@ -60,8 +60,10 @@ TableBasedTableBuilder::TableBasedTableBuilder(Header* header) } Table* TableBasedTableBuilder::GetTable() { - if (table_ == NULL) { - table_.Attach(down_cast(SubBuildTable(InternalReadData()))); + if (!table_) { + ReadableFontData* data = InternalReadData(); + if (data) + table_.Attach(down_cast(SubBuildTable(data))); } return table_; } -- cgit v1.2.3 From d651349cc7d61e667eadc186d5081a31a4339ee3 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 19 Oct 2016 14:52:17 -0700 Subject: Check for negative size in NameTable::NameAsBytes. Fixes https://crbug.com/654663 --- cpp/src/sfntly/table/core/name_table.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/table/core/name_table.cc b/cpp/src/sfntly/table/core/name_table.cc index 8d2f64f..c9fe468 100644 --- a/cpp/src/sfntly/table/core/name_table.cc +++ b/cpp/src/sfntly/table/core/name_table.cc @@ -469,12 +469,14 @@ int32_t NameTable::NameId(int32_t index) { void NameTable::NameAsBytes(int32_t index, ByteVector* b) { assert(b); - int32_t length = NameLength(index); b->clear(); + + int32_t length = NameLength(index); + if (length <= 0) + return; + b->resize(length); - if (length > 0) { - data_->ReadBytes(NameOffset(index), &((*b)[0]), 0, length); - } + data_->ReadBytes(NameOffset(index), &((*b)[0]), 0, length); } void NameTable::NameAsBytes(int32_t platform_id, -- cgit v1.2.3 From 1bc53e167ca0bee0c89a938fe69e5bdb86b28d6b Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 19 Oct 2016 14:53:24 -0700 Subject: Fix undefined shifts in ReadableFontData::ReadShort. Shifting a negative value is undefined behavior. Fixes https://crbug.com/655914 --- cpp/src/sfntly/data/readable_font_data.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/data/readable_font_data.cc b/cpp/src/sfntly/data/readable_font_data.cc index 93678ff..07a0db6 100644 --- a/cpp/src/sfntly/data/readable_font_data.cc +++ b/cpp/src/sfntly/data/readable_font_data.cc @@ -111,7 +111,9 @@ int32_t ReadableFontData::ReadShort(int32_t index) { int32_t b2 = ReadUByte(index + 1); if (b2 < 0) return kInvalidShort; - return ((b1 << 8 | b2) << 16) >> 16; + + uint32_t result = static_cast(b1) << 8 | b2; + return static_cast(result << 16) >> 16; } int32_t ReadableFontData::ReadUInt24(int32_t index) { -- cgit v1.2.3 From ebaa364dd0e270b6954331dc5ffb5fe4462de372 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 25 Oct 2016 18:32:02 -0700 Subject: Fix breakage from commit 083b02b1. While the previous commit fixed NULL pointer deferences, it also returned NULL pointers for some tables that needed to be set. As a result, sfntly failed to generate correct output, as seen in https://crbug.com/659006. --- cpp/src/sfntly/font.cc | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/font.cc b/cpp/src/sfntly/font.cc index 0c268cc..ca35048 100644 --- a/cpp/src/sfntly/font.cc +++ b/cpp/src/sfntly/font.cc @@ -400,7 +400,12 @@ static Table::Builder* GetBuilder(TableBuilderMap* builder_map, int32_t tag) { if (target == builder_map->end()) return NULL; - Table::Builder* builder = target->second.p_; + return target->second.p_; +} + +// Like GetBuilder(), but the returned Builder must be able to support reads. +static Table::Builder* GetReadBuilder(TableBuilderMap* builder_map, int32_t tag) { + Table::Builder* builder = GetBuilder(builder_map, tag); if (!builder || !builder->InternalReadData()) return NULL; @@ -408,46 +413,46 @@ static Table::Builder* GetBuilder(TableBuilderMap* builder_map, int32_t tag) { } void Font::Builder::InterRelateBuilders(TableBuilderMap* builder_map) { - Table::Builder* raw_head_builder = GetBuilder(builder_map, Tag::head); + Table::Builder* raw_head_builder = GetReadBuilder(builder_map, Tag::head); FontHeaderTableBuilderPtr header_table_builder; if (raw_head_builder != NULL) { - header_table_builder = - down_cast(raw_head_builder); + header_table_builder = + down_cast(raw_head_builder); } - Table::Builder* raw_hhea_builder = GetBuilder(builder_map, Tag::hhea); + Table::Builder* raw_hhea_builder = GetReadBuilder(builder_map, Tag::hhea); HorizontalHeaderTableBuilderPtr horizontal_header_builder; if (raw_head_builder != NULL) { - horizontal_header_builder = - down_cast(raw_hhea_builder); + horizontal_header_builder = + down_cast(raw_hhea_builder); } - Table::Builder* raw_maxp_builder = GetBuilder(builder_map, Tag::maxp); + Table::Builder* raw_maxp_builder = GetReadBuilder(builder_map, Tag::maxp); MaximumProfileTableBuilderPtr max_profile_builder; if (raw_maxp_builder != NULL) { - max_profile_builder = - down_cast(raw_maxp_builder); + max_profile_builder = + down_cast(raw_maxp_builder); } Table::Builder* raw_loca_builder = GetBuilder(builder_map, Tag::loca); LocaTableBuilderPtr loca_table_builder; if (raw_loca_builder != NULL) { - loca_table_builder = down_cast(raw_loca_builder); + loca_table_builder = down_cast(raw_loca_builder); } Table::Builder* raw_hmtx_builder = GetBuilder(builder_map, Tag::hmtx); HorizontalMetricsTableBuilderPtr horizontal_metrics_builder; if (raw_hmtx_builder != NULL) { - horizontal_metrics_builder = - down_cast(raw_hmtx_builder); + horizontal_metrics_builder = + down_cast(raw_hmtx_builder); } #if defined (SFNTLY_EXPERIMENTAL) Table::Builder* raw_hdmx_builder = GetBuilder(builder_map, Tag::hdmx); HorizontalDeviceMetricsTableBuilderPtr hdmx_table_builder; if (raw_hdmx_builder != NULL) { - hdmx_table_builder = - down_cast(raw_hdmx_builder); + hdmx_table_builder = + down_cast(raw_hdmx_builder); } #endif -- cgit v1.2.3 From 427f36e967318da60e86404a638bcecb10c97dbd Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Fri, 28 Oct 2016 17:41:01 -0700 Subject: Fix invalid cast found by Control Flow Integrity. Instead of casting RefCounted objects to type NoAddRefRelease, make AddRef() and Release() private methods that are only accessible to a limited number of friends. Fixes https://crbug.com/517959 --- cpp/src/sfntly/port/refcount.h | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'cpp') diff --git a/cpp/src/sfntly/port/refcount.h b/cpp/src/sfntly/port/refcount.h index eed5162..6353b08 100644 --- a/cpp/src/sfntly/port/refcount.h +++ b/cpp/src/sfntly/port/refcount.h @@ -99,22 +99,18 @@ namespace sfntly { +template +class Ptr; + class RefCount { public: // Make gcc -Wnon-virtual-dtor happy. virtual ~RefCount() {} - virtual size_t AddRef() const = 0; - virtual size_t Release() const = 0; -}; - -template -class NoAddRefRelease : public T { - public: - NoAddRefRelease(); - ~NoAddRefRelease(); - private: + template + friend class Ptr; + virtual size_t AddRef() const = 0; virtual size_t Release() const = 0; }; @@ -142,6 +138,7 @@ class RefCounted : virtual public RefCount { return *this; } + private: virtual size_t AddRef() const { size_t new_count = AtomicIncrement(&ref_count_); DEBUG_OUTPUT("A "); @@ -224,8 +221,8 @@ class Ptr { return *p_; // It can throw! } - NoAddRefRelease* operator->() const { - return (NoAddRefRelease*)p_; // It can throw! + T* operator->() const { + return p_; // It can throw! } bool operator!() const { -- cgit v1.2.3