From d71f05ca07eb5c7cfa4d6d23e5c1f2a800f52e8e Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Thu, 20 Jun 2019 23:35:23 -0400 Subject: Fix sign and conversion warnings (major) This makes all integer type conversions that have potential data loss explicit with calls that do range checks and raise an exception. After this commit, qpdf builds with no warnings when -Wsign-conversion -Wconversion is used with gcc or clang or when -W3 -Wd4800 is used with MSVC. This significantly reduces the likelihood of potential crashes from bogus integer values. There are some parts of the code that take int when they should take size_t or an offset. Such places would make qpdf not support files with more than 2^31 of something that usually wouldn't be so large. In the event that such a file shows up and is valid, at least qpdf would raise an error in the right spot so the issue could be legitimately addressed rather than failing in some weird way because of a silent overflow condition. --- libtests/aes.cc | 3 ++- libtests/bits.cc | 32 +++++++++++++++++--------------- libtests/dct_compress.cc | 4 ++-- libtests/predictors.cc | 7 +++++-- libtests/qintc.cc | 4 ++-- libtests/qtest/qintc/qintc.out | 2 +- libtests/qtest/qutil/qutil.out | 1 + libtests/qutil.cc | 3 ++- libtests/rc4.cc | 5 +++-- 9 files changed, 35 insertions(+), 26 deletions(-) (limited to 'libtests') diff --git a/libtests/aes.cc b/libtests/aes.cc index b5a63d9a..26906d46 100644 --- a/libtests/aes.cc +++ b/libtests/aes.cc @@ -1,6 +1,7 @@ #include #include #include +#include #include #include @@ -87,7 +88,7 @@ int main(int argc, char* argv[]) usage(); } - unsigned int hexkeylen = strlen(hexkey); + unsigned int hexkeylen = QIntC::to_uint(strlen(hexkey)); unsigned int keylen = hexkeylen / 2; FILE* infile = QUtil::safe_fopen(infilename, "rb"); diff --git a/libtests/bits.cc b/libtests/bits.cc index 3b274f93..1dfd42ed 100644 --- a/libtests/bits.cc +++ b/libtests/bits.cc @@ -1,6 +1,8 @@ #include #include #include +#include +#include #include #include #include @@ -12,8 +14,8 @@ #include "../libqpdf/bits.icc" static void -print_values(int byte_offset, unsigned int bit_offset, - unsigned int bits_available) +print_values(long long byte_offset, size_t bit_offset, + size_t bits_available) { std::cout << "byte offset = " << byte_offset << ", " << "bit offset = " << bit_offset << ", " @@ -22,11 +24,11 @@ print_values(int byte_offset, unsigned int bit_offset, static void test_read_bits(unsigned char const* buf, - unsigned char const*& p, unsigned int& bit_offset, - unsigned int& bits_available, int bits_wanted) + unsigned char const*& p, size_t& bit_offset, + size_t& bits_available, size_t bits_wanted) { unsigned long result = - read_bits(p, bit_offset, bits_available, bits_wanted); + QIntC::to_ulong(read_bits(p, bit_offset, bits_available, bits_wanted)); std::cout << "bits read: " << bits_wanted << ", result = " << result << std::endl; @@ -34,12 +36,12 @@ test_read_bits(unsigned char const* buf, } static void -test_write_bits(unsigned char& ch, unsigned int& bit_offset, unsigned long val, - int bits, Pl_Buffer* bp) +test_write_bits(unsigned char& ch, size_t& bit_offset, unsigned long val, + size_t bits, Pl_Buffer* bp) { write_bits(ch, bit_offset, val, bits, bp); - printf("ch = %02x, bit_offset = %d\n", - static_cast(ch), bit_offset); + std::cout << "ch = " << QUtil::uint_to_string_base(ch, 16, 2) + << ", bit_offset = " << bit_offset << std::endl; } static void @@ -51,10 +53,10 @@ print_buffer(Pl_Buffer* bp) size_t l = b->getSize(); for (unsigned long i = 0; i < l; ++i) { - printf("%02x%s", static_cast(p[i]), - (i == l - 1) ? "\n" : " "); + std::cout << QUtil::uint_to_string_base(p[i], 16, 2) + << ((i == l - 1) ? "\n" : " "); } - printf("\n"); + std::cout << std::endl; delete b; } @@ -71,8 +73,8 @@ test() }; unsigned char const* p = buf; - unsigned int bit_offset = 7; - unsigned int bits_available = 64; + size_t bit_offset = 7; + size_t bits_available = 64; // 11110:101 0:001010:1 01100101: 01111001 // 0:00:1:0010 10001001 01110101 01001:011 @@ -163,7 +165,7 @@ test() bw.writeBits(30UL, 5); bw.flush(); bw.flush(); - bw.writeBits(0xABUL, 8); + bw.writeBitsInt(0xAB, 8); bw.flush(); print_buffer(bp); bw.writeBitsSigned(-1, 3); // 111 diff --git a/libtests/dct_compress.cc b/libtests/dct_compress.cc index 65539582..590af57f 100644 --- a/libtests/dct_compress.cc +++ b/libtests/dct_compress.cc @@ -42,8 +42,8 @@ int main(int argc, char* argv[]) char* infilename = argv[1]; char* outfilename = argv[2]; - int width = QUtil::string_to_int(argv[3]); - int height = QUtil::string_to_int(argv[4]); + JDIMENSION width = QUtil::string_to_uint(argv[3]); + JDIMENSION height = QUtil::string_to_uint(argv[4]); char* colorspace = argv[5]; J_COLOR_SPACE cs = ((strcmp(colorspace, "rgb") == 0) ? JCS_RGB : diff --git a/libtests/predictors.cc b/libtests/predictors.cc index 47efa287..9342e22c 100644 --- a/libtests/predictors.cc +++ b/libtests/predictors.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -11,7 +12,7 @@ void run(char const* filename, char const* filter, bool encode, unsigned int columns, - int bits_per_sample, int samples_per_pixel) + unsigned int bits_per_sample, unsigned int samples_per_pixel) { FILE* in = QUtil::safe_fopen(filename, "rb"); FILE* o1 = QUtil::safe_fopen("out", "wb"); @@ -89,7 +90,9 @@ int main(int argc, char* argv[]) try { run(filename, filter, encode, - columns, bits_per_sample, samples_per_pixel); + QIntC::to_uint(columns), + QIntC::to_uint(bits_per_sample), + QIntC::to_uint(samples_per_pixel)); } catch (std::exception& e) { diff --git a/libtests/qintc.cc b/libtests/qintc.cc index 79fc6f53..95105aeb 100644 --- a/libtests/qintc.cc +++ b/libtests/qintc.cc @@ -48,10 +48,10 @@ int main() try_convert(true, QIntC::to_uint, ul2); try_convert(true, QIntC::to_offset, u1); try_convert(true, QIntC::to_offset, i1); - try_convert(false, QIntC::to_size, i1); + try_convert(false, QIntC::to_ulonglong, i1); try_convert(true, QIntC::to_char, i2); try_convert(true, QIntC::to_uchar, i2); - try_convert(false, QIntC::to_uchar, c1); + try_convert(false, QIntC::to_uchar, c1); return 0; } diff --git a/libtests/qtest/qintc/qintc.out b/libtests/qtest/qintc/qintc.out index 9b6d35d9..5c6479e2 100644 --- a/libtests/qtest/qintc/qintc.out +++ b/libtests/qtest/qintc/qintc.out @@ -7,7 +7,7 @@ QIntC::to_int(ul2): 12345 12345 PASSED QIntC::to_uint(ul2): 12345 12345 PASSED QIntC::to_offset(u1): 3141592653 3141592653 PASSED QIntC::to_offset(i1): -1153374643 -1153374643 PASSED -QIntC::to_size(i1): integer out of range converting -1153374643 from a 4-byte signed type to a 8-byte unsigned type PASSED +QIntC::to_ulonglong(i1): integer out of range converting -1153374643 from a 4-byte signed type to a 8-byte unsigned type PASSED QIntC::to_char(i2): 81 Q PASSED QIntC::to_uchar(i2): 81 Q PASSED QIntC::to_uchar(c1): integer out of range converting ÷ from a 1-byte signed type to a 1-byte unsigned type PASSED diff --git a/libtests/qtest/qutil/qutil.out b/libtests/qtest/qutil/qutil.out index 4d19617d..2f270226 100644 --- a/libtests/qtest/qutil/qutil.out +++ b/libtests/qtest/qutil/qutil.out @@ -12,6 +12,7 @@ 16059 37273 3ebb +5000093552 one 7 compare okay diff --git a/libtests/qutil.cc b/libtests/qutil.cc index 0e0a063b..900f0e6f 100644 --- a/libtests/qutil.cc +++ b/libtests/qutil.cc @@ -93,7 +93,8 @@ void string_conversion_test() << QUtil::double_to_string(.000123456, 5) << std::endl << QUtil::int_to_string_base(16059, 10) << std::endl << QUtil::int_to_string_base(16059, 8) << std::endl - << QUtil::int_to_string_base(16059, 16) << std::endl; + << QUtil::int_to_string_base(16059, 16) << std::endl + << QUtil::int_to_string_base(5000093552LL, 10) << std::endl; std::string embedded_null = "one"; embedded_null += '\0'; diff --git a/libtests/rc4.cc b/libtests/rc4.cc index 7a4d8bd9..f877666f 100644 --- a/libtests/rc4.cc +++ b/libtests/rc4.cc @@ -1,6 +1,7 @@ #include #include #include +#include #include #include @@ -18,7 +19,7 @@ int main(int argc, char* argv[]) char* hexkey = argv[1]; char* infilename = argv[2]; char* outfilename = argv[3]; - unsigned int hexkeylen = strlen(hexkey); + unsigned int hexkeylen = QIntC::to_uint(strlen(hexkey)); unsigned int keylen = hexkeylen / 2; unsigned char* key = new unsigned char[keylen + 1]; key[keylen] = '\0'; @@ -38,7 +39,7 @@ int main(int argc, char* argv[]) FILE* outfile = QUtil::safe_fopen(outfilename, "wb"); Pl_StdioFile* out = new Pl_StdioFile("stdout", outfile); // Use a small buffer size (64) for testing - Pl_RC4* rc4 = new Pl_RC4("rc4", out, key, keylen, 64); + Pl_RC4* rc4 = new Pl_RC4("rc4", out, key, QIntC::to_int(keylen), 64U); delete [] key; // 64 < buffer size < 512, buffer_size is not a power of 2 for testing -- cgit v1.2.3-54-g00ecf