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.

ChangeLog

@@ -1,9 +1,19 @@
 2019-06-20  Jay Berkenbilt  <[email protected]>
 
+	* Fix all integer sign and conversion warnings. This makes all
+	integer type conversions that have potential data loss explicit
+	with calls that do range checks and raise an exception.
+
 	* Change out_bufsize argument to Pl_Flate's constructor for int to
 	unsigned int for compatibility with underlying zlib
 	implementation.
 
+	* Change QPDFObjectHandle::pipeStreamData's encode_flags argument
+	from unsigned long to int since int is the underlying type of the
+	enumerated type values that are passed to it. This change should
+	be invisible to virtually all code unless you are compiling with
+	strict warning flags and explicitly casting to unsigned long.
+
 	* Add methods to QPDFObjectHandle to return the value of Integer
 	objects as int and unsigned int with range checking and fallback
 	behavior to avoid silent underflow/overflow conditions.

NOTICE.md

@@ -10,8 +10,6 @@ Versions of qpdf prior to version 7 were released under the terms of version 2.0
 
 The qpdf distribution includes a copy of [qtest](http://qtest.qbilt.org), which is released under the terms of the [version 2.0 of the Artistic license](https://opensource.org/licenses/Artistic-2.0), which can be found at https://opensource.org/licenses/Artistic-2.0.
 
-The standalone fuzz target runner (fuzz/standalone_fuzz_target_runner.cc) is copyright 2017 by Google and is also released under the Apache license, Version 2.0.
-
 The Rijndael encryption implementation used as the basis for AES encryption and decryption support comes from Philip J. Erdelsky's public domain implementation.  The files `libqpdf/rijndael.cc` and `libqpdf/qpdf/rijndael.h` remain in the public domain.  They were obtained from
 * http://www.efgh.com/software/rijndael.htm
 * http://www.efgh.com/software/rijndael.txt

examples/pdf-create.cc

@@ -14,6 +14,7 @@
 #include <qpdf/Pl_Buffer.hh>
 #include <qpdf/Pl_RunLength.hh>
 #include <qpdf/Pl_DCT.hh>
+#include <qpdf/QIntC.hh>
 #include <iostream>
 #include <string.h>
 #include <stdlib.h>
@@ -30,15 +31,15 @@ class ImageProvider: public QPDFObjectHandle::StreamDataProvider
     virtual ~ImageProvider();
     virtual void provideStreamData(int objid, int generation,
 				   Pipeline* pipeline);
-    int getWidth() const;
-    int getHeight() const;
+    size_t getWidth() const;
+    size_t getHeight() const;
 
   private:
-    int width;
-    int stripe_height;
+    size_t width;
+    size_t stripe_height;
     std::string color_space;
     std::string filter;
-    int n_stripes;
+    size_t n_stripes;
     std::vector<std::string> stripes;
     J_COLOR_SPACE j_color_space;
 };
@@ -88,13 +89,13 @@ ImageProvider::~ImageProvider()
 {
 }
 
-int
+size_t
 ImageProvider::getWidth() const
 {
     return width;
 }
 
-int
+size_t
 ImageProvider::getHeight() const
 {
     return stripe_height * n_stripes;
@@ -111,7 +112,8 @@ ImageProvider::provideStreamData(int objid, int generation,
     {
         p = new Pl_DCT(
             "image encoder", pipeline,
-            width, getHeight(), stripes[0].length(), j_color_space);
+            QIntC::to_uint(width), QIntC::to_uint(getHeight()),
+            QIntC::to_int(stripes[0].length()), j_color_space);
         to_delete.push_back(p);
     }
     else if (filter == "/RunLengthDecode")
@@ -121,9 +123,9 @@ ImageProvider::provideStreamData(int objid, int generation,
         to_delete.push_back(p);
     }
 
-    for (int i = 0; i < n_stripes; ++i)
+    for (size_t i = 0; i < n_stripes; ++i)
     {
-        for (int j = 0; j < width * stripe_height; ++j)
+        for (size_t j = 0; j < width * stripe_height; ++j)
         {
             p->write(
                 QUtil::unsigned_char_pointer(stripes[i].c_str()),
@@ -155,9 +157,9 @@ QPDFObjectHandle newName(std::string const& name)
     return QPDFObjectHandle::newName(name);
 }
 
-QPDFObjectHandle newInteger(int val)
+QPDFObjectHandle newInteger(size_t val)
 {
-    return QPDFObjectHandle::newInteger(val);
+    return QPDFObjectHandle::newInteger(QIntC::to_int(val));
 }
 
 void add_page(QPDFPageDocumentHelper& dh, QPDFObjectHandle font,
@@ -173,8 +175,8 @@ void add_page(QPDFPageDocumentHelper& dh, QPDFObjectHandle font,
     // compression.
     ImageProvider* p = new ImageProvider(color_space, filter);
     PointerHolder<QPDFObjectHandle::StreamDataProvider> provider(p);
-    int width = p->getWidth();
-    int height = p->getHeight();
+    size_t width = p->getWidth();
+    size_t height = p->getHeight();
     QPDFObjectHandle image = QPDFObjectHandle::newStream(&pdf);
     image.replaceDict(QPDFObjectHandle::parse(
                           "<<"
@@ -335,8 +337,8 @@ static void check(char const* filename,
                 unsigned int mismatches = 0;
                 int tolerance = (
                     desired_filter == "/DCTDecode" ? 10 : 0);
-                unsigned int threshold = (
-                    desired_filter == "/DCTDecode" ? len / 40 : 0);
+                size_t threshold = (
+                    desired_filter == "/DCTDecode" ? len / 40U : 0);
                 for (size_t i = 0; i < len; ++i)
                 {
                     int delta = actual_bytes[i] - desired_bytes[i];

examples/pdf-double-page-size.cc

@@ -34,7 +34,7 @@ static void doubleBoxSize(QPDFObjectHandle& page, char const* box_name)
 				 " is not an array of four elements");
     }
     std::vector<QPDFObjectHandle> doubled;
-    for (unsigned int i = 0; i < 4; ++i)
+    for (int i = 0; i < 4; ++i)
     {
 	doubled.push_back(
 	    QPDFObjectHandle::newReal(

examples/pdf-invert-images.cc

@@ -7,6 +7,7 @@
 #include <qpdf/QUtil.hh>
 #include <qpdf/Buffer.hh>
 #include <qpdf/QPDFWriter.hh>
+#include <qpdf/QIntC.hh>
 
 static char const* whoami = 0;
 
@@ -56,7 +57,7 @@ ImageInverter::provideStreamData(int objid, int generation,
     unsigned char ch;
     for (size_t i = 0; i < size; ++i)
     {
-	ch = static_cast<unsigned char>(0xff) - buf[i];
+	ch = QIntC::to_uchar(0xff - buf[i]);
 	pipeline->write(&ch, 1);
     }
     pipeline->finish();

examples/pdf-parse-content.cc

@@ -6,6 +6,7 @@
 #include <qpdf/QPDFPageDocumentHelper.hh>
 #include <qpdf/QPDFPageObjectHelper.hh>
 #include <qpdf/QUtil.hh>
+#include <qpdf/QIntC.hh>
 
 static char const* whoami = 0;
 
@@ -72,12 +73,12 @@ int main(int argc, char* argv[])
 	pdf.processFile(filename);
         std::vector<QPDFPageObjectHelper> pages =
             QPDFPageDocumentHelper(pdf).getAllPages();
-        if ((pageno < 1) || (static_cast<size_t>(pageno) > pages.size()))
+        if ((pageno < 1) || (QIntC::to_size(pageno) > pages.size()))
         {
             usage();
         }
 
-        QPDFPageObjectHelper& page = pages.at(pageno-1);
+        QPDFPageObjectHelper& page = pages.at(QIntC::to_size(pageno-1));
         ParserCallbacks cb;
         page.parsePageContents(&cb);
     }

examples/pdf-split-pages.cc

@@ -8,6 +8,7 @@
 #include <qpdf/QPDFPageDocumentHelper.hh>
 #include <qpdf/QPDFWriter.hh>
 #include <qpdf/QUtil.hh>
+#include <qpdf/QIntC.hh>
 
 #include <iostream>
 #include <stdlib.h>
@@ -24,7 +25,8 @@ static void process(char const* whoami,
     inpdf.processFile(infile);
     std::vector<QPDFPageObjectHelper> pages =
         QPDFPageDocumentHelper(inpdf).getAllPages();
-    int pageno_len = QUtil::int_to_string(pages.size()).length();
+    int pageno_len =
+        QIntC::to_int(QUtil::uint_to_string(pages.size()).length());
     int pageno = 0;
     for (std::vector<QPDFPageObjectHelper>::iterator iter = pages.begin();
          iter != pages.end(); ++iter)

fuzz/standalone_fuzz_target_runner.cc

@@ -1,34 +1,46 @@
-// Copyright 2017 Google Inc. All Rights Reserved.
-// Licensed under the Apache License, Version 2.0 (the "License");
-
-// Except for formatting, comments, and portability, this was copied
-// from projects/example/my-api-repo/standalone_fuzz_target_runner.cpp
-// in https://github.com/oss-fuzz
-
-#include <cassert>
+#include <qpdf/QUtil.hh>
+#include <qpdf/PointerHolder.hh>
+#include <qpdf/QIntC.hh>
 #include <iostream>
-#include <fstream>
-#include <vector>
+#include <string>
 
 extern "C" int LLVMFuzzerTestOneInput(unsigned char const* data, size_t size);
 
+static void read_file_into_memory(
+    char const* filename,
+    PointerHolder<unsigned char>& file_buf, size_t& size)
+{
+    FILE* f = QUtil::safe_fopen(filename, "rb");
+    fseek(f, 0, SEEK_END);
+    size = QIntC::to_size(QUtil::tell(f));
+    fseek(f, 0, SEEK_SET);
+    file_buf = PointerHolder<unsigned char>(true, new unsigned char[size]);
+    unsigned char* buf_p = file_buf.getPointer();
+    size_t bytes_read = 0;
+    size_t len = 0;
+    while ((len = fread(buf_p + bytes_read, 1, size - bytes_read, f)) > 0)
+    {
+        bytes_read += len;
+    }
+    if (bytes_read != size)
+    {
+        throw std::runtime_error(
+            std::string("failure reading file ") + filename +
+            " into memory: read " +
+            QUtil::uint_to_string(bytes_read) + "; wanted " +
+            QUtil::uint_to_string(size));
+    }
+    fclose(f);
+}
+
 int main(int argc, char **argv)
 {
     for (int i = 1; i < argc; i++)
     {
-        std::ifstream in(argv[i]);
-        in.seekg(0, in.end);
-        size_t length = in.tellg();
-        in.seekg (0, in.beg);
-        std::cout << "checking " << argv[i] << std::endl;
-        // Allocate exactly length bytes so that we reliably catch
-        // buffer overflows.
-        std::vector<char> bytes(length);
-        in.read(bytes.data(), bytes.size());
-        assert(in);
-        LLVMFuzzerTestOneInput(
-            reinterpret_cast<unsigned char const*>(bytes.data()),
-            bytes.size());
+        PointerHolder<unsigned char> file_buf;
+        size_t size = 0;
+        read_file_into_memory(argv[i], file_buf, size);
+        LLVMFuzzerTestOneInput(file_buf.getPointer(), size);
         std::cout << argv[i] << " successful" << std::endl;
     }
     return 0;

include/qpdf/BufferInputSource.hh

@@ -54,6 +54,8 @@ class BufferInputSource: public InputSource
     virtual void unreadCh(char ch);
 
   private:
+    qpdf_offset_t const bufSizeAsOffset() const;
+
     bool own_memory;
     std::string description;
     Buffer* buf;

include/qpdf/Pl_Count.hh

@@ -48,6 +48,8 @@ class Pl_Count: public Pipeline
     unsigned char getLastChar() const;
 
   private:
+    // Must be qpdf_offset_t, not size_t, to handle writing more than
+    // size_t can handle.
     qpdf_offset_t count;
     unsigned char last_char;
 };

include/qpdf/QPDF.hh

@@ -32,6 +32,7 @@
 #include <iostream>
 #include <vector>
 
+#include <qpdf/QIntC.hh>
 #include <qpdf/QPDFExc.hh>
 #include <qpdf/QPDFObjectHandle.hh>
 #include <qpdf/QPDFObjGen.hh>
@@ -859,7 +860,7 @@ class QPDF
     bool pipeForeignStreamData(
         PointerHolder<ForeignStreamData>,
         Pipeline*,
-        unsigned long encode_flags,
+        int encode_flags,
         qpdf_stream_decode_level_e decode_level);
     static bool pipeStreamData(PointerHolder<QPDF::EncryptionParameters> encp,
                                PointerHolder<InputSource> file,
@@ -1253,7 +1254,7 @@ class QPDF
     void dumpHPageOffset();
     void dumpHSharedObject();
     void dumpHGeneric(HGeneric&);
-    int adjusted_offset(int offset);
+    qpdf_offset_t adjusted_offset(qpdf_offset_t offset);
     QPDFObjectHandle objGenToIndirect(QPDFObjGen const&);
     void calculateLinearizationData(
 	std::map<int, int> const& object_stream_data);
@@ -1297,6 +1298,20 @@ class QPDF
 				  std::set<QPDFObjGen>& visited, bool top);
     void filterCompressedObjects(std::map<int, int> const& object_stream_data);
 
+    // Type conversion helper methods
+    template<typename T> static qpdf_offset_t toO(T const& i)
+    {
+        return QIntC::to_offset(i);
+    }
+    template<typename T> static size_t toS(T const& i)
+    {
+        return QIntC::to_size(i);
+    }
+    template<typename T> static int toI(T const& i)
+    {
+        return QIntC::to_int(i);
+    }
+
     class Members
     {
         friend class QPDF;

include/qpdf/QPDFWriter.hh

@@ -467,7 +467,7 @@ class QPDFWriter
 
     enum trailer_e { t_normal, t_lin_first, t_lin_second };
 
-    int bytesNeeded(unsigned long long n);
+    unsigned int bytesNeeded(long long n);
     void writeBinary(unsigned long long val, unsigned int bytes);
     void writeString(std::string const& str);
     void writeBuffer(PointerHolder<Buffer>&);
@@ -483,10 +483,8 @@ class QPDFWriter
     void writeTrailer(trailer_e which, int size,
 		      bool xref_stream, qpdf_offset_t prev,
                       int linearization_pass);
-    void unparseObject(QPDFObjectHandle object, int level,
-		       unsigned int flags);
-    void unparseObject(QPDFObjectHandle object, int level,
-		       unsigned int flags,
+    void unparseObject(QPDFObjectHandle object, int level, int flags);
+    void unparseObject(QPDFObjectHandle object, int level, int flags,
 		       // for stream dictionaries
 		       size_t stream_length, bool compress);
     void unparseChild(QPDFObjectHandle child, int level, int flags);
@@ -510,7 +508,7 @@ class QPDFWriter
 	char const* user_password, char const* owner_password,
 	int V, int R, int key_len, std::set<int>& bits_to_clear);
     void setEncryptionParametersInternal(
-	int V, int R, int key_len, long P,
+	int V, int R, int key_len, int P,
 	std::string const& O, std::string const& U,
 	std::string const& OE, std::string const& UE, std::string const& Perms,
 	std::string const& id1, std::string const& user_password,
@@ -554,7 +552,7 @@ class QPDFWriter
         qpdf_offset_t hint_length,
         bool skip_compression,
         int linearization_pass);
-    int calculateXrefStreamPadding(int xref_bytes);
+    int calculateXrefStreamPadding(qpdf_offset_t xref_bytes);
 
     // When filtering subsections, push additional pipelines to the
     // stack.  When ready to switch, activate the pipeline stack.