From 3b8632b51c11a80a612aeb4a7d1e72d636c48c7e Mon Sep 17 00:00:00 2001 From: Tomas Popela Date: Fri, 12 Oct 2018 10:44:36 +0200 Subject: [PATCH 1/4] Fix uninitialized member of Table struct Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/font.cc:88: error[uninitStructMember]: Uninitialized struct member: table.flavor Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/font.cc:88: error[uninitStructMember]: Uninitialized struct member: table.num_tables Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.checksum Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.data Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.dst_data Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.dst_length Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.dst_offset Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.flag_byte Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.length Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.offset Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.reuse_of Error: CPPCHECK_WARNING (CWE-909): woff2-1.0.2/src/woff2_enc.cc:349: error[uninitStructMember]: Uninitialized struct member: table.src_offset --- src/font.cc | 2 +- src/woff2_enc.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/font.cc b/src/font.cc index a45153e..e1cf497 100644 --- a/src/font.cc +++ b/src/font.cc @@ -66,7 +66,7 @@ bool ReadTrueTypeFont(Buffer* file, const uint8_t* data, size_t len, std::map intervals; for (uint16_t i = 0; i < font->num_tables; ++i) { - Font::Table table; + Font::Table table = {}; table.flag_byte = 0; table.reuse_of = NULL; if (!file->ReadU32(&table.tag) || diff --git a/src/woff2_enc.cc b/src/woff2_enc.cc index ec00878..ed9bd06 100644 --- a/src/woff2_enc.cc +++ b/src/woff2_enc.cc @@ -331,7 +331,7 @@ bool ConvertTTFToWOFF2(const uint8_t *data, size_t length, return false; } - Table table; + Table table = {}; table.tag = src_table.tag; table.flags = src_table.flag_byte; table.src_length = src_table.length; From 027c59bec7a59f9aed2fed6e940e013b8a9c30fc Mon Sep 17 00:00:00 2001 From: Tomas Popela Date: Fri, 12 Oct 2018 10:47:24 +0200 Subject: [PATCH 2/4] Don't compare unsigned int with signed Error: COMPILER_WARNING: woff2-1.0.2/src/font.cc: scope_hint: In function 'int woff2::NumGlyphs(const woff2::Font&)' woff2-1.0.2/src/font.cc:330:26: warning: comparison of integer expressions of different signedness: 'const uint32_t' {aka 'const unsigned int'} and 'int' [-Wsign-compare] if (loca_table->length < loca_record_size) { ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~ 328| int index_fmt = IndexFormat(font); 329| int loca_record_size = (index_fmt == 0 ? 2 : 4); 330|-> if (loca_table->length < loca_record_size) { 331| return 0; 332| } Error: COMPILER_WARNING: woff2-1.0.2/src/normalize.cc:15: included_from: Included from here. woff2-1.0.2/src/normalize.cc: scope_hint: In function 'bool woff2::{anonymous}::MakeEditableBuffer(woff2::Font*, int)' woff2-1.0.2/src/normalize.cc:100:24: warning: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Wsign-compare] if (PREDICT_FALSE(sz > table->length)) { ~~~^~~~~~~~~~~~~~~ woff2-1.0.2/src/port.h:48:44: note: in definition of macro 'PREDICT_FALSE' #define PREDICT_FALSE(x) (__builtin_expect(x, 0)) ^ 98| uint8_t* buf = &table->buffer[0]; 99| memcpy(buf, table->data, table->length); 100|-> if (PREDICT_FALSE(sz > table->length)) { 101| memset(buf + table->length, 0, sz - table->length); 102| } Error: COMPILER_WARNING: woff2-1.0.2/src/woff2_info.cc: scope_hint: In function 'int main(int, char**)' woff2-1.0.2/src/woff2_info.cc:125:24: warning: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Wsign-compare] for (auto i = 0; i < numFonts; i++) { ~~^~~~~~~~~~ 123| printf("CollectionHeader 0x%08x %d fonts\n", version, numFonts); 124| 125|-> for (auto i = 0; i < numFonts; i++) { 126| uint32_t numTables, flavor; 127| if (!woff2::Read255UShort(&file, &numTables)) return 1; Error: COMPILER_WARNING: woff2-1.0.2/src/woff2_info.cc:131:26: warning: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Wsign-compare] for (auto j = 0; j < numTables; j++) { ~~^~~~~~~~~~~ 129| printf("CollectionFontEntry %d flavor 0x%08x %d tables\n", i, flavor, 130| numTables); 131|-> for (auto j = 0; j < numTables; j++) { 132| uint32_t table_idx; 133| if (!woff2::Read255UShort(&file, &table_idx)) return 1; --- src/font.cc | 2 +- src/normalize.cc | 2 +- src/woff2_info.cc | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/font.cc b/src/font.cc index e1cf497..0e9f5bf 100644 --- a/src/font.cc +++ b/src/font.cc @@ -326,7 +326,7 @@ int NumGlyphs(const Font& font) { return 0; } int index_fmt = IndexFormat(font); - int loca_record_size = (index_fmt == 0 ? 2 : 4); + uint32_t loca_record_size = (index_fmt == 0 ? 2 : 4); if (loca_table->length < loca_record_size) { return 0; } diff --git a/src/normalize.cc b/src/normalize.cc index 6685e08..d54e817 100644 --- a/src/normalize.cc +++ b/src/normalize.cc @@ -97,7 +97,7 @@ bool MakeEditableBuffer(Font* font, int tableTag) { table->buffer.resize(sz); uint8_t* buf = &table->buffer[0]; memcpy(buf, table->data, table->length); - if (PREDICT_FALSE(sz > table->length)) { + if (PREDICT_FALSE(static_cast(sz) > table->length)) { memset(buf + table->length, 0, sz - table->length); } table->data = buf; diff --git a/src/woff2_info.cc b/src/woff2_info.cc index 2b51adc..8ec9d36 100644 --- a/src/woff2_info.cc +++ b/src/woff2_info.cc @@ -122,13 +122,13 @@ int main(int argc, char **argv) { if (!woff2::Read255UShort(&file, &numFonts)) return 1; printf("CollectionHeader 0x%08x %d fonts\n", version, numFonts); - for (auto i = 0; i < numFonts; i++) { + for (auto i = 0u; i < numFonts; i++) { uint32_t numTables, flavor; if (!woff2::Read255UShort(&file, &numTables)) return 1; if (!file.ReadU32(&flavor)) return 1; printf("CollectionFontEntry %d flavor 0x%08x %d tables\n", i, flavor, numTables); - for (auto j = 0; j < numTables; j++) { + for (auto j = 0u; j < numTables; j++) { uint32_t table_idx; if (!woff2::Read255UShort(&file, &table_idx)) return 1; if (table_idx >= table_tags.size()) return 1; From 7b5e1dcbd82bf5fc0581b6106b35bbfc11617236 Mon Sep 17 00:00:00 2001 From: Tomas Popela Date: Fri, 12 Oct 2018 10:50:35 +0200 Subject: [PATCH 3/4] Uninitialized members in the Glyph class Error: UNINIT_CTOR (CWE-456): woff2-1.0.2/src/glyph.h:28: member_decl: Class member declaration for "x_min". woff2-1.0.2/src/glyph.h:25: uninit_member: Non-static class member "x_min" is not initialized in this constructor nor in any functions that it calls. woff2-1.0.2/src/glyph.h:29: member_decl: Class member declaration for "x_max". woff2-1.0.2/src/glyph.h:25: uninit_member: Non-static class member "x_max" is not initialized in this constructor nor in any functions that it calls. woff2-1.0.2/src/glyph.h:30: member_decl: Class member declaration for "y_min". woff2-1.0.2/src/glyph.h:25: uninit_member: Non-static class member "y_min" is not initialized in this constructor nor in any functions that it calls. woff2-1.0.2/src/glyph.h:31: member_decl: Class member declaration for "y_max". woff2-1.0.2/src/glyph.h:25: uninit_member: Non-static class member "y_max" is not initialized in this constructor nor in any functions that it calls. woff2-1.0.2/src/glyph.h:35: member_decl: Class member declaration for "instructions_data". woff2-1.0.2/src/glyph.h:25: uninit_member: Non-static class member "instructions_data" is not initialized in this constructor nor in any functions that it calls. woff2-1.0.2/src/glyph.h:46: member_decl: Class member declaration for "composite_data". woff2-1.0.2/src/glyph.h:25: uninit_member: Non-static class member "composite_data" is not initialized in this constructor nor in any functions that it calls. woff2-1.0.2/src/glyph.h:48: member_decl: Class member declaration for "have_instructions". woff2-1.0.2/src/glyph.h:25: uninit_member: Non-static class member "have_instructions" is not initialized in this constructor nor in any functions that it calls. 23| class Glyph { 24| public: 25|-> Glyph() : instructions_size(0), composite_data_size(0) {} 26| 27| // Bounding box. --- src/glyph.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/glyph.h b/src/glyph.h index f24056f..e870188 100644 --- a/src/glyph.h +++ b/src/glyph.h @@ -22,17 +22,17 @@ namespace woff2 { // is around. class Glyph { public: - Glyph() : instructions_size(0), composite_data_size(0) {} + Glyph() {} // Bounding box. - int16_t x_min; - int16_t x_max; - int16_t y_min; - int16_t y_max; + int16_t x_min = 0; + int16_t x_max = 0; + int16_t y_min = 0; + int16_t y_max = 0; // Instructions. - uint16_t instructions_size; - const uint8_t* instructions_data; + uint16_t instructions_size = 0; + const uint8_t* instructions_data = 0; // Data model for simple glyphs. struct Point { @@ -43,9 +43,9 @@ class Glyph { std::vector > contours; // Data for composite glyphs. - const uint8_t* composite_data; - uint32_t composite_data_size; - bool have_instructions; + const uint8_t* composite_data = 0; + uint32_t composite_data_size = 0; + bool have_instructions = false; }; // Parses the glyph from the given data. Returns false on parsing failure or From 0d1d30a67ab379ba0c2a1c00064135bf70762803 Mon Sep 17 00:00:00 2001 From: Tomas Popela Date: Fri, 12 Oct 2018 10:51:53 +0200 Subject: [PATCH 4/4] Remove unused variables Error: COMPILER_WARNING: woff2-1.0.2/src/normalize.cc: scope_hint: In function 'bool woff2::FixChecksums(woff2::Font*)' woff2-1.0.2/src/normalize.cc:216:12: warning: variable 'head_checksum' set but not used [-Wunused-but-set-variable] uint32_t head_checksum = 0; ^~~~~~~~~~~~~ 214| StoreU32(0, &offset, head_buf); 215| uint32_t file_checksum = 0; 216|-> uint32_t head_checksum = 0; 217| for (auto& i : font->tables) { 218| Font::Table* table = &i.second; Error: CLANG_WARNING: woff2-1.0.2/src/normalize.cc:226:7: note: Value stored to 'head_checksum' is never read head_checksum = table->checksum; ^ ~~~~~~~~~~~~~~~ 224| 225| if (table->tag == kHeadTableTag) { 226|-> head_checksum = table->checksum; 227| } 228| } Error: CLANG_WARNING: woff2-1.0.2/src/woff2_dec.cc:306:3: note: Value stored to 'offset' is never read offset = Store16(dst, offset, y_max); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~ 304| offset = Store16(dst, offset, y_min); 305| offset = Store16(dst, offset, x_max); 306|-> offset = Store16(dst, offset, y_max); 307| } 308| Error: CLANG_WARNING: woff2-1.0.2/src/woff2_enc.cc:339:22: note: Value stored to 'transformed_data' during its initialization is never read const uint8_t* transformed_data = src_table.data; ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~ 337| table.src_length = src_table.length; 338| table.transform_length = src_table.length; 339|-> const uint8_t* transformed_data = src_table.data; 340| const Font::Table* transformed_table = 341| font.FindTable(src_table.tag ^ 0x80808080); Error: COMPILER_WARNING: woff2-1.0.2/src/woff2_enc.cc: scope_hint: In function 'bool woff2::ConvertTTFToWOFF2(const uint8_t*, size_t, uint8_t*, size_t*, const woff2::WOFF2Params&)' woff2-1.0.2/src/woff2_enc.cc:339:22: warning: variable 'transformed_data' set but not used [-Wunused-but-set-variable] const uint8_t* transformed_data = src_table.data; ^~~~~~~~~~~~~~~~ 337| table.src_length = src_table.length; 338| table.transform_length = src_table.length; 339|-> const uint8_t* transformed_data = src_table.data; 340| const Font::Table* transformed_table = 341| font.FindTable(src_table.tag ^ 0x80808080); Error: CLANG_WARNING: woff2-1.0.2/src/woff2_enc.cc:346:9: note: Value stored to 'transformed_data' is never read transformed_data = transformed_table->data; ^ ~~~~~~~~~~~~~~~~~~~~~~~ 344| table.flags |= kWoff2FlagsTransform; 345| table.transform_length = transformed_table->length; 346|-> transformed_data = transformed_table->data; 347| 348| } Error: CLANG_WARNING: woff2-1.0.2/src/woff2_enc.cc:426:18: note: Value stored to 'table_length' during its initialization is never read uint32_t table_length = ^~~~~~~~~~~~ 424| uint32_t table_offset = 425| table.IsReused() ? table.reuse_of->offset : table.offset; 426|-> uint32_t table_length = 427| table.IsReused() ? table.reuse_of->length : table.length; 428| std::pair tag_offset(table.tag, table_offset); Error: COMPILER_WARNING: woff2-1.0.2/src/woff2_enc.cc:426:18: warning: unused variable 'table_length' [-Wunused-variable] uint32_t table_length = ^~~~~~~~~~~~ 424| uint32_t table_offset = 425| table.IsReused() ? table.reuse_of->offset : table.offset; 426|-> uint32_t table_length = 427| table.IsReused() ? table.reuse_of->length : table.length; 428| std::pair tag_offset(table.tag, table_offset); --- src/normalize.cc | 5 ----- src/woff2_dec.cc | 2 +- src/woff2_enc.cc | 5 ----- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/normalize.cc b/src/normalize.cc index d54e817..a819074 100644 --- a/src/normalize.cc +++ b/src/normalize.cc @@ -213,7 +213,6 @@ bool FixChecksums(Font* font) { size_t offset = 8; StoreU32(0, &offset, head_buf); uint32_t file_checksum = 0; - uint32_t head_checksum = 0; for (auto& i : font->tables) { Font::Table* table = &i.second; if (table->IsReused()) { @@ -221,10 +220,6 @@ bool FixChecksums(Font* font) { } table->checksum = ComputeULongSum(table->data, table->length); file_checksum += table->checksum; - - if (table->tag == kHeadTableTag) { - head_checksum = table->checksum; - } } file_checksum += ComputeHeaderChecksum(*font); diff --git a/src/woff2_dec.cc b/src/woff2_dec.cc index 25e18c6..442baa5 100644 --- a/src/woff2_dec.cc +++ b/src/woff2_dec.cc @@ -316,7 +316,7 @@ void ComputeBbox(unsigned int n_points, const Point* points, uint8_t* dst) { offset = Store16(dst, offset, x_min); offset = Store16(dst, offset, y_min); offset = Store16(dst, offset, x_max); - offset = Store16(dst, offset, y_max); + Store16(dst, offset, y_max); } diff --git a/src/woff2_enc.cc b/src/woff2_enc.cc index ed9bd06..c0598f8 100644 --- a/src/woff2_enc.cc +++ b/src/woff2_enc.cc @@ -336,15 +336,12 @@ bool ConvertTTFToWOFF2(const uint8_t *data, size_t length, table.flags = src_table.flag_byte; table.src_length = src_table.length; table.transform_length = src_table.length; - const uint8_t* transformed_data = src_table.data; const Font::Table* transformed_table = font.FindTable(src_table.tag ^ 0x80808080); if (transformed_table != NULL) { table.flags = transformed_table->flag_byte; table.flags |= kWoff2FlagsTransform; table.transform_length = transformed_table->length; - transformed_data = transformed_table->data; - } tables.push_back(table); } @@ -423,8 +420,6 @@ bool ConvertTTFToWOFF2(const uint8_t *data, size_t length, // for reused tables, only the original has an updated offset uint32_t table_offset = table.IsReused() ? table.reuse_of->offset : table.offset; - uint32_t table_length = - table.IsReused() ? table.reuse_of->length : table.length; std::pair tag_offset(table.tag, table_offset); if (index_by_tag_offset.find(tag_offset) == index_by_tag_offset.end()) { #ifdef FONT_COMPRESSION_BIN