Fix unsafe buffer issues in fxcodec/gif
-- Convert to std::array<> and pdfium::span<>
-- Mark remaining regions UNSAFE_TODO().
Bug: 42271175
Change-Id: I31c18c081f678f5aa1fafda4ec6806664ea14a17
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119713
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
diff --git a/core/fxcodec/gif/cfx_gifcontext.cpp b/core/fxcodec/gif/cfx_gifcontext.cpp
index 0be2a41..71c9e66 100644
--- a/core/fxcodec/gif/cfx_gifcontext.cpp
+++ b/core/fxcodec/gif/cfx_gifcontext.cpp
@@ -4,17 +4,13 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "core/fxcodec/gif/cfx_gifcontext.h"
#include <stdint.h>
#include <string.h>
#include <algorithm>
+#include <array>
#include <iterator>
#include <utility>
@@ -27,7 +23,7 @@
namespace {
-constexpr int32_t kGifInterlaceStep[4] = {8, 8, 4, 2};
+constexpr std::array<const int32_t, 4> kGifInterlaceStep = {{8, 8, 4, 2}};
} // namespace
@@ -247,8 +243,10 @@
SaveDecodingStatus(GIF_D_STATUS_IMG_DATA);
img_row_offset_ += img_row_avail_size_;
img_row_avail_size_ = gif_img_row_bytes - img_row_offset_;
- LZWDecompressor::Status ret = lzw_decompressor_->Decode(
- gif_image->row_buffer.data() + img_row_offset_, &img_row_avail_size_);
+ auto img_row_span = pdfium::make_span(gif_image->row_buffer)
+ .subspan(img_row_offset_, img_row_avail_size_);
+ LZWDecompressor::Status ret =
+ lzw_decompressor_->Decode(img_row_span.data(), &img_row_avail_size_);
if (ret == LZWDecompressor::Status::kError) {
DecodingFailureAtTailCleanup(gif_image);
return GifDecoder::Status::kError;
@@ -278,9 +276,10 @@
SaveDecodingStatus(GIF_D_STATUS_IMG_DATA);
img_row_offset_ += img_row_avail_size_;
img_row_avail_size_ = gif_img_row_bytes - img_row_offset_;
- ret = lzw_decompressor_->Decode(
- gif_image->row_buffer.data() + img_row_offset_,
- &img_row_avail_size_);
+ img_row_span = pdfium::make_span(gif_image->row_buffer)
+ .subspan(img_row_offset_, img_row_avail_size_);
+ ret = lzw_decompressor_->Decode(img_row_span.data(),
+ &img_row_avail_size_);
}
}
@@ -303,9 +302,10 @@
img_row_offset_ = 0;
img_row_avail_size_ = gif_img_row_bytes;
- ret = lzw_decompressor_->Decode(
- gif_image->row_buffer.data() + img_row_offset_,
- &img_row_avail_size_);
+ img_row_span = pdfium::make_span(gif_image->row_buffer)
+ .subspan(img_row_offset_, img_row_avail_size_);
+ ret = lzw_decompressor_->Decode(img_row_span.data(),
+ &img_row_avail_size_);
}
if (ret == LZWDecompressor::Status::kError) {
diff --git a/core/fxcodec/gif/lzw_decompressor.cpp b/core/fxcodec/gif/lzw_decompressor.cpp
index 45fd974..85f0a29 100644
--- a/core/fxcodec/gif/lzw_decompressor.cpp
+++ b/core/fxcodec/gif/lzw_decompressor.cpp
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "core/fxcodec/gif/lzw_decompressor.h"
#include <algorithm>
@@ -16,6 +11,7 @@
#include <type_traits>
#include <utility>
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_safe_types.h"
#include "core/fxcrt/numerics/safe_conversions.h"
#include "core/fxcrt/ptr_util.h"
@@ -63,11 +59,11 @@
FX_SAFE_UINT32 i = 0;
if (decompressed_next_ != 0) {
size_t extracted_size =
- ExtractData(pdfium::make_span(dest_buf, *dest_size));
+ ExtractData(UNSAFE_TODO(pdfium::make_span(dest_buf, *dest_size)));
if (decompressed_next_ != 0)
return Status::kInsufficientDestSize;
- dest_buf += extracted_size;
+ UNSAFE_TODO(dest_buf += extracted_size);
i += extracted_size;
}
@@ -80,7 +76,7 @@
if (bits_left_ > 31)
return Status::kError;
- FX_SAFE_UINT32 safe_code = *next_in_++;
+ FX_SAFE_UINT32 safe_code = UNSAFE_TODO(*next_in_++);
safe_code <<= bits_left_;
safe_code |= code_store_;
if (!safe_code.IsValid())
@@ -127,12 +123,12 @@
}
code_old_ = code;
- size_t extracted_size = ExtractData(
- pdfium::make_span(dest_buf, (*dest_size - i).ValueOrDie()));
+ size_t extracted_size = ExtractData(UNSAFE_TODO(
+ pdfium::make_span(dest_buf, (*dest_size - i).ValueOrDie())));
if (decompressed_next_ != 0) {
return Status::kInsufficientDestSize;
}
- dest_buf += extracted_size;
+ UNSAFE_TODO(dest_buf += extracted_size);
i += extracted_size;
}
}
@@ -195,9 +191,9 @@
return 0;
}
size_t copy_size = std::min(dest_span.size(), decompressed_next_);
- std::reverse_copy(decompressed_.data() + decompressed_next_ - copy_size,
- decompressed_.data() + decompressed_next_,
- dest_span.data());
+ UNSAFE_TODO(std::reverse_copy(
+ decompressed_.data() + decompressed_next_ - copy_size,
+ decompressed_.data() + decompressed_next_, dest_span.data()));
decompressed_next_ -= copy_size;
return copy_size;
}
diff --git a/core/fxcodec/gif/lzw_decompressor.h b/core/fxcodec/gif/lzw_decompressor.h
index a58b25e..131d327 100644
--- a/core/fxcodec/gif/lzw_decompressor.h
+++ b/core/fxcodec/gif/lzw_decompressor.h
@@ -10,6 +10,7 @@
#include <stddef.h>
#include <stdint.h>
+#include <array>
#include <memory>
#include "core/fxcodec/gif/cfx_gif.h"
@@ -71,7 +72,7 @@
uint32_t avail_in_ = 0;
uint8_t bits_left_ = 0;
uint32_t code_store_ = 0;
- CodeEntry code_table_[GIF_MAX_LZW_CODE];
+ std::array<CodeEntry, GIF_MAX_LZW_CODE> code_table_;
};
} // namespace fxcodec
