Commit ad8081daf597b8f46696d5ddae82770ab419ad82
1 parent
9a095c5c
Fix fuzz issue 15442 (overflow checking in BufferInputSource)
Showing
3 changed files
with
27 additions
and
11 deletions
fuzz/qpdf_extra/15442.fuzz
0 → 100644
include/qpdf/BufferInputSource.hh
| @@ -54,7 +54,7 @@ class BufferInputSource: public InputSource | @@ -54,7 +54,7 @@ class BufferInputSource: public InputSource | ||
| 54 | virtual void unreadCh(char ch); | 54 | virtual void unreadCh(char ch); |
| 55 | 55 | ||
| 56 | private: | 56 | private: |
| 57 | - qpdf_offset_t const bufSizeAsOffset() const; | 57 | + static void range_check(qpdf_offset_t cur, qpdf_offset_t delta); |
| 58 | 58 | ||
| 59 | class Members | 59 | class Members |
| 60 | { | 60 | { |
| @@ -72,6 +72,7 @@ class BufferInputSource: public InputSource | @@ -72,6 +72,7 @@ class BufferInputSource: public InputSource | ||
| 72 | std::string description; | 72 | std::string description; |
| 73 | Buffer* buf; | 73 | Buffer* buf; |
| 74 | qpdf_offset_t cur_offset; | 74 | qpdf_offset_t cur_offset; |
| 75 | + qpdf_offset_t max_offset; | ||
| 75 | }; | 76 | }; |
| 76 | 77 | ||
| 77 | PointerHolder<Members> m; | 78 | PointerHolder<Members> m; |
libqpdf/BufferInputSource.cc
| @@ -3,6 +3,8 @@ | @@ -3,6 +3,8 @@ | ||
| 3 | #include <string.h> | 3 | #include <string.h> |
| 4 | #include <stdexcept> | 4 | #include <stdexcept> |
| 5 | #include <algorithm> | 5 | #include <algorithm> |
| 6 | +#include <limits> | ||
| 7 | +#include <sstream> | ||
| 6 | 8 | ||
| 7 | BufferInputSource::Members::Members(bool own_memory, | 9 | BufferInputSource::Members::Members(bool own_memory, |
| 8 | std::string const& description, | 10 | std::string const& description, |
| @@ -10,7 +12,8 @@ BufferInputSource::Members::Members(bool own_memory, | @@ -10,7 +12,8 @@ BufferInputSource::Members::Members(bool own_memory, | ||
| 10 | own_memory(own_memory), | 12 | own_memory(own_memory), |
| 11 | description(description), | 13 | description(description), |
| 12 | buf(buf), | 14 | buf(buf), |
| 13 | - cur_offset(0) | 15 | + cur_offset(0), |
| 16 | + max_offset(buf ? QIntC::to_offset(buf->getSize()) : 0) | ||
| 14 | { | 17 | { |
| 15 | } | 18 | } |
| 16 | 19 | ||
| @@ -29,6 +32,7 @@ BufferInputSource::BufferInputSource(std::string const& description, | @@ -29,6 +32,7 @@ BufferInputSource::BufferInputSource(std::string const& description, | ||
| 29 | m(new Members(true, description, 0)) | 32 | m(new Members(true, description, 0)) |
| 30 | { | 33 | { |
| 31 | this->m->buf = new Buffer(contents.length()); | 34 | this->m->buf = new Buffer(contents.length()); |
| 35 | + this->m->max_offset = QIntC::to_offset(this->m->buf->getSize()); | ||
| 32 | unsigned char* bp = this->m->buf->getBuffer(); | 36 | unsigned char* bp = this->m->buf->getBuffer(); |
| 33 | memcpy(bp, contents.c_str(), contents.length()); | 37 | memcpy(bp, contents.c_str(), contents.length()); |
| 34 | } | 38 | } |
| @@ -41,12 +45,6 @@ BufferInputSource::~BufferInputSource() | @@ -41,12 +45,6 @@ BufferInputSource::~BufferInputSource() | ||
| 41 | } | 45 | } |
| 42 | } | 46 | } |
| 43 | 47 | ||
| 44 | -qpdf_offset_t const | ||
| 45 | -BufferInputSource::bufSizeAsOffset() const | ||
| 46 | -{ | ||
| 47 | - return QIntC::to_offset(this->m->buf->getSize()); | ||
| 48 | -} | ||
| 49 | - | ||
| 50 | qpdf_offset_t | 48 | qpdf_offset_t |
| 51 | BufferInputSource::findAndSkipNextEOL() | 49 | BufferInputSource::findAndSkipNextEOL() |
| 52 | { | 50 | { |
| @@ -54,7 +52,7 @@ BufferInputSource::findAndSkipNextEOL() | @@ -54,7 +52,7 @@ BufferInputSource::findAndSkipNextEOL() | ||
| 54 | { | 52 | { |
| 55 | throw std::logic_error("INTERNAL ERROR: BufferInputSource offset < 0"); | 53 | throw std::logic_error("INTERNAL ERROR: BufferInputSource offset < 0"); |
| 56 | } | 54 | } |
| 57 | - qpdf_offset_t end_pos = bufSizeAsOffset(); | 55 | + qpdf_offset_t end_pos = this->m->max_offset; |
| 58 | if (this->m->cur_offset >= end_pos) | 56 | if (this->m->cur_offset >= end_pos) |
| 59 | { | 57 | { |
| 60 | this->last_offset = end_pos; | 58 | this->last_offset = end_pos; |
| @@ -103,6 +101,20 @@ BufferInputSource::tell() | @@ -103,6 +101,20 @@ BufferInputSource::tell() | ||
| 103 | } | 101 | } |
| 104 | 102 | ||
| 105 | void | 103 | void |
| 104 | +BufferInputSource::range_check(qpdf_offset_t cur, qpdf_offset_t delta) | ||
| 105 | +{ | ||
| 106 | + if ((delta > 0) && | ||
| 107 | + ((std::numeric_limits<qpdf_offset_t>::max() - cur) < delta)) | ||
| 108 | + { | ||
| 109 | + std::ostringstream msg; | ||
| 110 | + msg << "seeking forward from " << cur | ||
| 111 | + << " by " << delta | ||
| 112 | + << " would cause an overflow of the offset type"; | ||
| 113 | + throw std::range_error(msg.str()); | ||
| 114 | + } | ||
| 115 | +} | ||
| 116 | + | ||
| 117 | +void | ||
| 106 | BufferInputSource::seek(qpdf_offset_t offset, int whence) | 118 | BufferInputSource::seek(qpdf_offset_t offset, int whence) |
| 107 | { | 119 | { |
| 108 | switch (whence) | 120 | switch (whence) |
| @@ -112,10 +124,12 @@ BufferInputSource::seek(qpdf_offset_t offset, int whence) | @@ -112,10 +124,12 @@ BufferInputSource::seek(qpdf_offset_t offset, int whence) | ||
| 112 | break; | 124 | break; |
| 113 | 125 | ||
| 114 | case SEEK_END: | 126 | case SEEK_END: |
| 115 | - this->m->cur_offset = bufSizeAsOffset() + offset; | 127 | + range_check(this->m->max_offset, offset); |
| 128 | + this->m->cur_offset = this->m->max_offset + offset; | ||
| 116 | break; | 129 | break; |
| 117 | 130 | ||
| 118 | case SEEK_CUR: | 131 | case SEEK_CUR: |
| 132 | + range_check(this->m->cur_offset, offset); | ||
| 119 | this->m->cur_offset += offset; | 133 | this->m->cur_offset += offset; |
| 120 | break; | 134 | break; |
| 121 | 135 | ||
| @@ -145,7 +159,7 @@ BufferInputSource::read(char* buffer, size_t length) | @@ -145,7 +159,7 @@ BufferInputSource::read(char* buffer, size_t length) | ||
| 145 | { | 159 | { |
| 146 | throw std::logic_error("INTERNAL ERROR: BufferInputSource offset < 0"); | 160 | throw std::logic_error("INTERNAL ERROR: BufferInputSource offset < 0"); |
| 147 | } | 161 | } |
| 148 | - qpdf_offset_t end_pos = bufSizeAsOffset(); | 162 | + qpdf_offset_t end_pos = this->m->max_offset; |
| 149 | if (this->m->cur_offset >= end_pos) | 163 | if (this->m->cur_offset >= end_pos) |
| 150 | { | 164 | { |
| 151 | this->last_offset = end_pos; | 165 | this->last_offset = end_pos; |