From 286cb90883bda53296b44b063b4a1bf4efdeb837 Mon Sep 17 00:00:00 2001 From: SteveGilvarry Date: Tue, 16 Jun 2026 21:55:06 +1000 Subject: [PATCH 1/2] fix: compute camera frame buffer sizes in size_t to avoid int overflow The frame buffer size calculations in the libVNC and libVLC camera backends multiplied 16/32-bit operands and only widened the result to size_t afterwards, so the multiplication itself could overflow before the conversion. Flagged by CodeQL cpp/integer-multiplication-cast-to-long (alerts 282, 145, 85). The VNC framebuffer dimensions are advertised by the remote server (uint16_t), making the inputs externally influenced. Cast the first operand to size_t so each product is computed in size_t: - zm_libvnc_camera.cpp: SwScale::Convert src size (framebufferWidth * framebufferHeight * 4) and dest size (width * height * colours) - zm_libvnc_camera.cpp: resize() bufferSize, changed from int to size_t (not converted to a larger type, so CodeQL did not flag it, but the same overflow applied); Debug format updated to %zu - zm_libvlc_camera.cpp: Image::Assign size (width * height * mBpp) Verified: both translation units compile with -Werror -fsyntax-only. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/zm_libvlc_camera.cpp | 2 +- src/zm_libvnc_camera.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/zm_libvlc_camera.cpp b/src/zm_libvlc_camera.cpp index f66651a87..ed4c75dbe 100644 --- a/src/zm_libvlc_camera.cpp +++ b/src/zm_libvlc_camera.cpp @@ -307,7 +307,7 @@ int LibvlcCamera::Capture(std::shared_ptr &zm_packet) { return 0; mLibvlcData.mutex.lock(); - zm_packet->image->Assign(width, height, colours, subpixelorder, mLibvlcData.buffer, width * height * mBpp); + zm_packet->image->Assign(width, height, colours, subpixelorder, mLibvlcData.buffer, static_cast(width) * height * mBpp); zm_packet->packet->stream_index = mVideoStreamId; zm_packet->stream = mVideoStream; mLibvlcData.mutex.unlock(); diff --git a/src/zm_libvnc_camera.cpp b/src/zm_libvnc_camera.cpp index 4ffec93ec..568d84398 100644 --- a/src/zm_libvnc_camera.cpp +++ b/src/zm_libvnc_camera.cpp @@ -73,11 +73,11 @@ static rfbBool resize(rfbClient* client) { av_free(client->frameBuffer); } - int bufferSize = 4*client->width*client->height; + size_t bufferSize = static_cast(client->width) * client->height * 4; // libVNC doesn't do alignment or padding in each line //SWScale::GetBufferSize(AV_PIX_FMT_RGBA, client->width, client->height); client->frameBuffer = (uint8_t *)av_malloc(bufferSize); - Debug(1, "Allocing new frame buffer %dx%d = %d", client->width, client->height, bufferSize); + Debug(1, "Allocing new frame buffer %dx%d = %zu", client->width, client->height, bufferSize); return TRUE; } @@ -239,9 +239,9 @@ int VncCamera::Capture(std::shared_ptr &zm_packet) { // Image buffer (WriteBuffer), which is always align-32. int rc = scale.Convert( mVncData.buffer, - mRfb->si.framebufferWidth * mRfb->si.framebufferHeight * 4, + static_cast(mRfb->si.framebufferWidth) * mRfb->si.framebufferHeight * 4, directbuffer, - width * height * colours, + static_cast(width) * height * colours, AV_PIX_FMT_RGBA, mImgPixFmt, mRfb->si.framebufferWidth, From 45099e53a764ee9ac59fd715503c9341b9c3a1c5 Mon Sep 17 00:00:00 2001 From: SteveGilvarry Date: Thu, 18 Jun 2026 07:54:15 +1000 Subject: [PATCH 2/2] fix: address review feedback on camera buffer overflow fix - zm_libvnc_camera.cpp resize(): return FALSE and log if av_malloc fails, rather than returning TRUE with a null frameBuffer (libVNC would then write into it and crash). Matters more now that size_t sizing can request large allocations for server-advertised dimensions. - zm_libvnc_camera.cpp: compute the buffer sizes in the scale Debug() in size_t with %zu, so the logged sizes can't overflow int and match the values passed to SWScale::Convert. - zm_libvlc_camera: widen LibvlcPrivateData::bufferSize to size_t and compute it in size_t in PrimeCapture, so the allocation itself can't overflow. Image::Assign now passes the same stored bufferSize used for the allocation, so the read size can't exceed the buffer. Widen the compare loop index to size_t to match. Verified: both translation units compile with -Werror -fsyntax-only. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/zm_libvlc_camera.cpp | 6 +++--- src/zm_libvlc_camera.h | 2 +- src/zm_libvnc_camera.cpp | 10 +++++++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/zm_libvlc_camera.cpp b/src/zm_libvlc_camera.cpp index ed4c75dbe..255a93056 100644 --- a/src/zm_libvlc_camera.cpp +++ b/src/zm_libvlc_camera.cpp @@ -80,7 +80,7 @@ void LibvlcUnlockBuffer(void* opaque, void* picture, void *const *planes) { LibvlcPrivateData* data = reinterpret_cast(opaque); bool newFrame = false; - for( unsigned int i=0; i < data->bufferSize; i++ ) { + for( size_t i=0; i < data->bufferSize; i++ ) { if ( data->buffer[i] != data->prevBuffer[i] ) { newFrame = true; break; @@ -277,7 +277,7 @@ int LibvlcCamera::PrimeCapture() { (*libvlc_video_set_format_f)(mLibvlcMediaPlayer, mTargetChroma.c_str(), width, height, width * mBpp); (*libvlc_video_set_callbacks_f)(mLibvlcMediaPlayer, &LibvlcLockBuffer, &LibvlcUnlockBuffer, nullptr, &mLibvlcData); - mLibvlcData.bufferSize = width * height * mBpp; + mLibvlcData.bufferSize = static_cast(width) * height * mBpp; // Libvlc wants 32 byte alignment for images (should in theory do this for all image lines) mLibvlcData.buffer = (uint8_t*)zm_mallocaligned(64, mLibvlcData.bufferSize); mLibvlcData.prevBuffer = (uint8_t*)zm_mallocaligned(64, mLibvlcData.bufferSize); @@ -307,7 +307,7 @@ int LibvlcCamera::Capture(std::shared_ptr &zm_packet) { return 0; mLibvlcData.mutex.lock(); - zm_packet->image->Assign(width, height, colours, subpixelorder, mLibvlcData.buffer, static_cast(width) * height * mBpp); + zm_packet->image->Assign(width, height, colours, subpixelorder, mLibvlcData.buffer, mLibvlcData.bufferSize); zm_packet->packet->stream_index = mVideoStreamId; zm_packet->stream = mVideoStream; mLibvlcData.mutex.unlock(); diff --git a/src/zm_libvlc_camera.h b/src/zm_libvlc_camera.h index 8d925bc22..1a7c45fab 100644 --- a/src/zm_libvlc_camera.h +++ b/src/zm_libvlc_camera.h @@ -36,7 +36,7 @@ struct LibvlcPrivateData { uint8_t* buffer; uint8_t* prevBuffer; time_t prevTime; - uint32_t bufferSize; + size_t bufferSize; std::mutex mutex; bool newImage; diff --git a/src/zm_libvnc_camera.cpp b/src/zm_libvnc_camera.cpp index 568d84398..eaf970477 100644 --- a/src/zm_libvnc_camera.cpp +++ b/src/zm_libvnc_camera.cpp @@ -77,6 +77,10 @@ static rfbBool resize(rfbClient* client) { // libVNC doesn't do alignment or padding in each line //SWScale::GetBufferSize(AV_PIX_FMT_RGBA, client->width, client->height); client->frameBuffer = (uint8_t *)av_malloc(bufferSize); + if (!client->frameBuffer) { + Error("Failed to allocate %zu byte frame buffer for %dx%d", bufferSize, client->width, client->height); + return FALSE; + } Debug(1, "Allocing new frame buffer %dx%d = %zu", client->width, client->height, bufferSize); return TRUE; @@ -225,10 +229,10 @@ int VncCamera::Capture(std::shared_ptr &zm_packet) { zm_packet->stream = mVideoStream; uint8_t *directbuffer = zm_packet->image->WriteBuffer(width, height, colours, subpixelorder); - Debug(1, "scale src %p, %d, dest %p %d %d %dx%d %dx%d", mVncData.buffer, - mRfb->si.framebufferWidth * mRfb->si.framebufferHeight * 4, + Debug(1, "scale src %p, %zu, dest %p %zu %d %dx%d %dx%d", mVncData.buffer, + static_cast(mRfb->si.framebufferWidth) * mRfb->si.framebufferHeight * 4, directbuffer, - width * height * colours, + static_cast(width) * height * colours, mImgPixFmt, mRfb->si.framebufferWidth, mRfb->si.framebufferHeight,