harden untrusted-input size handling in TIFF read and raw-TCP frames
Two memory-safety/robustness fixes for input whose size is attacker- or peer-controlled: - ReadTIFF: a large IMAGELENGTH could overflow scanline_bytes * lines, undersizing the buffer that TIFFReadScanline then writes past. Guard the product against overflow before resize and reuse scanline_bytes in the loop. - Raw-TCP image path: an uncapped header payload_size drove a huge resize() that took down the receive thread (wedging the writer-facing acceptor). Add JFJOCH_TCP_MAX_PAYLOAD_SIZE and reject oversized frames at the single receive-loop choke point on both the pusher and puller sides. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -8,6 +8,11 @@
|
||||
constexpr uint32_t JFJOCH_TCP_MAGIC = 0x4A464A54; // JFJT
|
||||
constexpr uint32_t JFJOCH_TCP_VERSION = 3;
|
||||
|
||||
// Upper bound on a single frame's payload. A receiver rejects any header claiming more
|
||||
// before allocating for it: an uncapped payload_size would otherwise drive a huge
|
||||
// resize() and take down the receiving thread. 1 GiB is far above any real image frame.
|
||||
constexpr uint64_t JFJOCH_TCP_MAX_PAYLOAD_SIZE = static_cast<uint64_t>(1) << 30;
|
||||
|
||||
enum class TCPFrameType : uint16_t {
|
||||
START = 1,
|
||||
DATA = 2,
|
||||
|
||||
@@ -320,6 +320,13 @@ void TCPImagePuller::ReceiverThread() {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (frame.header.payload_size > JFJOCH_TCP_MAX_PAYLOAD_SIZE) {
|
||||
logger.Error("Oversized TCP frame payload, reconnecting to " + addr);
|
||||
CloseSocket();
|
||||
std::this_thread::sleep_for(std::chrono::milliseconds(20));
|
||||
continue;
|
||||
}
|
||||
|
||||
const auto frame_type = static_cast<TCPFrameType>(frame.header.type);
|
||||
|
||||
// Respond to keepalive ping with a keepalive pong
|
||||
|
||||
@@ -432,6 +432,12 @@ void TCPStreamPusher::PersistentAckThread(Connection* c) {
|
||||
break;
|
||||
}
|
||||
|
||||
if (h.payload_size > JFJOCH_TCP_MAX_PAYLOAD_SIZE) {
|
||||
c->broken = true;
|
||||
logger.Error("Oversized payload on persistent connection, socket " + std::to_string(c->socket_number));
|
||||
break;
|
||||
}
|
||||
|
||||
// Any well-formed frame from the peer is a sign of life.
|
||||
c->last_peer_activity_ns.store(SteadyNowNs(), std::memory_order_relaxed);
|
||||
|
||||
|
||||
+10
-3
@@ -3,6 +3,7 @@
|
||||
|
||||
#include <tiffio.h>
|
||||
#include <tiffio.hxx>
|
||||
#include <cstdint>
|
||||
#include <cstring>
|
||||
#include <sstream>
|
||||
|
||||
@@ -129,13 +130,19 @@ CompressedImage ReadTIFF(const std::string &s, std::vector<uint8_t> &buffer) {
|
||||
if (cols == 0 || lines == 0 || elem_size_bytes == 0)
|
||||
throw JFJochException(JFJochExceptionCategory::TIFFGeneratorError,"Size wrong");
|
||||
|
||||
if (cols * elem_size_bytes != TIFFScanlineSize(tiff))
|
||||
const size_t scanline_bytes = static_cast<size_t>(cols) * elem_size_bytes;
|
||||
if (scanline_bytes != TIFFScanlineSize(tiff))
|
||||
throw JFJochException(JFJochExceptionCategory::TIFFGeneratorError, "TIFFScanlineSize mismatch");
|
||||
|
||||
buffer.resize(static_cast<size_t>(cols) * static_cast<size_t>(lines) * elem_size_bytes);
|
||||
// Guard the buffer size against overflow: a huge IMAGELENGTH could otherwise wrap
|
||||
// scanline_bytes * lines to a small value, undersizing the buffer that TIFFReadScanline writes into.
|
||||
if (lines > SIZE_MAX / scanline_bytes)
|
||||
throw JFJochException(JFJochExceptionCategory::TIFFGeneratorError, "TIFF image too large");
|
||||
|
||||
buffer.resize(scanline_bytes * lines);
|
||||
|
||||
for (uint32_t i = 0; i < lines; i++) {
|
||||
if (TIFFReadScanline(tiff, buffer.data() + static_cast<size_t>(i) * cols * elem_size_bytes, i, 0) < 0)
|
||||
if (TIFFReadScanline(tiff, buffer.data() + static_cast<size_t>(i) * scanline_bytes, i, 0) < 0)
|
||||
throw JFJochException(JFJochExceptionCategory::TIFFGeneratorError, "TIFFReadScanline error");
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user