Commit f1c774f13fac105a904b1901bfbdc2f892ebccf9
1 parent
973edb4f
Refactor QPDF::processXRefStream
Tune pointer arithmetic.
Showing
1 changed file
with
10 additions
and
5 deletions
libqpdf/QPDF.cc
| ... | ... | @@ -1002,6 +1002,7 @@ QPDF::processXRefStream(qpdf_offset_t xref_offset, QPDFObjectHandle& xref_obj) |
| 1002 | 1002 | } |
| 1003 | 1003 | unsigned long long max_num_entries = static_cast<unsigned long long>(-1) / entry_size; |
| 1004 | 1004 | |
| 1005 | + // Process /Index entry | |
| 1005 | 1006 | std::vector<long long> indx; |
| 1006 | 1007 | if (Index_obj.isArray()) { |
| 1007 | 1008 | int n_index = Index_obj.getArrayNItems(); |
| ... | ... | @@ -1025,6 +1026,7 @@ QPDF::processXRefStream(qpdf_offset_t xref_offset, QPDFObjectHandle& xref_obj) |
| 1025 | 1026 | } |
| 1026 | 1027 | QTC::TC("qpdf", "QPDF xref /Index is array", n_index == 2 ? 0 : 1); |
| 1027 | 1028 | } else { |
| 1029 | + // We have already validayed the /Index key. | |
| 1028 | 1030 | QTC::TC("qpdf", "QPDF xref /Index is null"); |
| 1029 | 1031 | long long size = dict.getKey("/Size").getIntValue(); |
| 1030 | 1032 | indx.push_back(0); |
| ... | ... | @@ -1033,6 +1035,11 @@ QPDF::processXRefStream(qpdf_offset_t xref_offset, QPDFObjectHandle& xref_obj) |
| 1033 | 1035 | |
| 1034 | 1036 | size_t num_entries = 0; |
| 1035 | 1037 | for (size_t i = 1; i < indx.size(); i += 2) { |
| 1038 | + // We are guarding against the possibility of num_entries * entry_size overflowing. | |
| 1039 | + // We are not checking that entries are in ascending order as required by the spec, which | |
| 1040 | + // probably should generate a warning. We are also not checking that for each subsection | |
| 1041 | + // first object number + number of entries <= /Size. The spec requires us to ignore object | |
| 1042 | + // number > /Size. | |
| 1036 | 1043 | if (indx.at(i) > QIntC::to_longlong(max_num_entries - num_entries)) { |
| 1037 | 1044 | throw damagedPDF( |
| 1038 | 1045 | "xref stream", |
| ... | ... | @@ -1070,13 +1077,11 @@ QPDF::processXRefStream(qpdf_offset_t xref_offset, QPDFObjectHandle& xref_obj) |
| 1070 | 1077 | bool saw_first_compressed_object = false; |
| 1071 | 1078 | |
| 1072 | 1079 | // Actual size vs. expected size check above ensures that we will not overflow any buffers here. |
| 1073 | - // We know that entry_size * num_entries is equal to the size of the buffer. | |
| 1074 | - unsigned char const* data = bp->getBuffer(); | |
| 1080 | + // We know that entry_size * num_entries is less or equal to the size of the buffer. | |
| 1081 | + auto p = bp->getBuffer(); | |
| 1075 | 1082 | for (size_t i = 0; i < num_entries; ++i) { |
| 1076 | 1083 | // Read this entry |
| 1077 | - unsigned char const* entry = data + (entry_size * i); | |
| 1078 | 1084 | qpdf_offset_t fields[3]; |
| 1079 | - unsigned char const* p = entry; | |
| 1080 | 1085 | for (int j = 0; j < 3; ++j) { |
| 1081 | 1086 | fields[j] = 0; |
| 1082 | 1087 | if ((j == 0) && (W[0] == 0)) { |
| ... | ... | @@ -1085,7 +1090,7 @@ QPDF::processXRefStream(qpdf_offset_t xref_offset, QPDFObjectHandle& xref_obj) |
| 1085 | 1090 | } |
| 1086 | 1091 | for (int k = 0; k < W[j]; ++k) { |
| 1087 | 1092 | fields[j] <<= 8; |
| 1088 | - fields[j] += toI(*p++); | |
| 1093 | + fields[j] |= *p++; | |
| 1089 | 1094 | } |
| 1090 | 1095 | } |
| 1091 | 1096 | ... | ... |