[fix] validate macOS input display state before injecting events

src/device_controller/keyboard/mac/keyboard_capturer.cpp

@@ -310,6 +310,10 @@ int KeyboardCapturer::SendKeyboardCommand(int key_code, bool is_down,
     if (IsFunctionKey(cg_key_code) && !is_down) {
       CGEventRef fn_release_event =
           CGEventCreateKeyboardEvent(NULL, fn_key_code_, false);
+      if (!fn_release_event) {
+        LOG_ERROR("CGEventCreateKeyboardEvent failed for fn release");
+        return -1;
+      }
       CGEventPost(kCGHIDEventTap, fn_release_event);
       CFRelease(fn_release_event);
     }

src/device_controller/mouse/mac/mouse_controller.cpp

@@ -1,6 +1,7 @@
 #include "mouse_controller.h"
 
 #include <ApplicationServices/ApplicationServices.h>
+#include <algorithm>
 
 #include "rd_log.h"
 
@@ -20,85 +21,101 @@ int MouseController::Destroy() { return 0; }
 
 int MouseController::SendMouseCommand(RemoteAction remote_action,
                                       int display_index) {
+  if (remote_action.type != ControlType::mouse) {
+    return 0;
+  }
+
+  if (display_index < 0 ||
+      display_index >= static_cast<int>(display_info_list_.size())) {
+    LOG_WARN("Mouse command skipped, invalid display_index={}, displays={}",
+             display_index, display_info_list_.size());
+    return -1;
+  }
+
+  const DisplayInfo& display_info = display_info_list_[display_index];
+  if (display_info.width <= 0 || display_info.height <= 0) {
+    LOG_WARN("Mouse command skipped, invalid display geometry: {}x{}",
+             display_info.width, display_info.height);
+    return -1;
+  }
+
+  const float normalized_x = std::clamp(remote_action.m.x, 0.0f, 1.0f);
+  const float normalized_y = std::clamp(remote_action.m.y, 0.0f, 1.0f);
   int mouse_pos_x =
-      remote_action.m.x * display_info_list_[display_index].width +
-      display_info_list_[display_index].left;
+      normalized_x * display_info.width + display_info.left;
   int mouse_pos_y =
-      remote_action.m.y * display_info_list_[display_index].height +
-      display_info_list_[display_index].top;
+      normalized_y * display_info.height + display_info.top;
 
-  if (remote_action.type == ControlType::mouse) {
-    CGEventRef mouse_event = nullptr;
-    CGEventType mouse_type;
-    CGMouseButton mouse_button;
-    CGPoint mouse_point = CGPointMake(mouse_pos_x, mouse_pos_y);
+  CGEventRef mouse_event = nullptr;
+  CGEventType mouse_type;
+  CGMouseButton mouse_button;
+  CGPoint mouse_point = CGPointMake(mouse_pos_x, mouse_pos_y);
 
-    switch (remote_action.m.flag) {
-      case MouseFlag::left_down:
-        mouse_type = kCGEventLeftMouseDown;
-        left_dragging_ = true;
-        mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
-                                              kCGMouseButtonLeft);
-        break;
-      case MouseFlag::left_up:
-        mouse_type = kCGEventLeftMouseUp;
-        left_dragging_ = false;
-        mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
-                                              kCGMouseButtonLeft);
-        break;
-      case MouseFlag::right_down:
-        mouse_type = kCGEventRightMouseDown;
-        right_dragging_ = true;
-        mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
-                                              kCGMouseButtonRight);
-        break;
-      case MouseFlag::right_up:
-        mouse_type = kCGEventRightMouseUp;
-        right_dragging_ = false;
-        mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
-                                              kCGMouseButtonRight);
-        break;
-      case MouseFlag::middle_down:
-        mouse_type = kCGEventOtherMouseDown;
-        mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
-                                              kCGMouseButtonCenter);
-        break;
-      case MouseFlag::middle_up:
-        mouse_type = kCGEventOtherMouseUp;
-        mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
-                                              kCGMouseButtonCenter);
-        break;
-      case MouseFlag::wheel_vertical:
-        mouse_event = CGEventCreateScrollWheelEvent(
-            NULL, kCGScrollEventUnitLine, 2, remote_action.m.s, 0);
-        break;
-      case MouseFlag::wheel_horizontal:
-        mouse_event = CGEventCreateScrollWheelEvent(
-            NULL, kCGScrollEventUnitLine, 2, 0, remote_action.m.s);
-        break;
-      default:
-        if (left_dragging_) {
-          mouse_type = kCGEventLeftMouseDragged;
-          mouse_button = kCGMouseButtonLeft;
-        } else if (right_dragging_) {
-          mouse_type = kCGEventRightMouseDragged;
-          mouse_button = kCGMouseButtonRight;
-        } else {
-          mouse_type = kCGEventMouseMoved;
-          mouse_button = kCGMouseButtonLeft;
-        }
+  switch (remote_action.m.flag) {
+    case MouseFlag::left_down:
+      mouse_type = kCGEventLeftMouseDown;
+      left_dragging_ = true;
+      mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
+                                            kCGMouseButtonLeft);
+      break;
+    case MouseFlag::left_up:
+      mouse_type = kCGEventLeftMouseUp;
+      left_dragging_ = false;
+      mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
+                                            kCGMouseButtonLeft);
+      break;
+    case MouseFlag::right_down:
+      mouse_type = kCGEventRightMouseDown;
+      right_dragging_ = true;
+      mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
+                                            kCGMouseButtonRight);
+      break;
+    case MouseFlag::right_up:
+      mouse_type = kCGEventRightMouseUp;
+      right_dragging_ = false;
+      mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
+                                            kCGMouseButtonRight);
+      break;
+    case MouseFlag::middle_down:
+      mouse_type = kCGEventOtherMouseDown;
+      mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
+                                            kCGMouseButtonCenter);
+      break;
+    case MouseFlag::middle_up:
+      mouse_type = kCGEventOtherMouseUp;
+      mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
+                                            kCGMouseButtonCenter);
+      break;
+    case MouseFlag::wheel_vertical:
+      mouse_event = CGEventCreateScrollWheelEvent(
+          NULL, kCGScrollEventUnitLine, 2, remote_action.m.s, 0);
+      break;
+    case MouseFlag::wheel_horizontal:
+      mouse_event = CGEventCreateScrollWheelEvent(
+          NULL, kCGScrollEventUnitLine, 2, 0, remote_action.m.s);
+      break;
+    default:
+      if (left_dragging_) {
+        mouse_type = kCGEventLeftMouseDragged;
+        mouse_button = kCGMouseButtonLeft;
+      } else if (right_dragging_) {
+        mouse_type = kCGEventRightMouseDragged;
+        mouse_button = kCGMouseButtonRight;
+      } else {
+        mouse_type = kCGEventMouseMoved;
+        mouse_button = kCGMouseButtonLeft;
+      }
 
-        mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
-                                              mouse_button);
-        break;
-    }
+      mouse_event = CGEventCreateMouseEvent(NULL, mouse_type, mouse_point,
+                                            mouse_button);
+      break;
+  }
 
-    if (mouse_event) {
-      CGEventPost(kCGHIDEventTap, mouse_event);
-      CFRelease(mouse_event);
-    }
+  if (mouse_event) {
+    CGEventPost(kCGHIDEventTap, mouse_event);
+    CFRelease(mouse_event);
   }
 
   return 0;
 }
-}  // namespace crossdesk
+}  // namespace crossdesk

src/gui/render.cpp

@@ -764,11 +764,16 @@ int Render::StartSpeakerCapturer() {
   }
 
   if (speaker_capturer_) {
-    speaker_capturer_->Start();
+    const int ret = speaker_capturer_->Start();
+    if (ret != 0) {
+      LOG_ERROR("Start speaker capturer failed: {}", ret);
+      return ret;
+    }
     start_speaker_capturer_ = true;
+    return 0;
   }
 
-  return 0;
+  return -1;
 }
 
 int Render::StopSpeakerCapturer() {
@@ -1149,8 +1154,9 @@ void Render::UpdateInteractions() {
   }
 
   if (start_speaker_capturer_ && !speaker_capturer_is_started_) {
-    StartSpeakerCapturer();
-    speaker_capturer_is_started_ = true;
+    if (0 == StartSpeakerCapturer()) {
+      speaker_capturer_is_started_ = true;
+    }
   } else if (!start_speaker_capturer_ && speaker_capturer_is_started_) {
     StopSpeakerCapturer();
     speaker_capturer_is_started_ = false;

src/gui/render.h

@@ -553,8 +553,8 @@ class Render {
   std::string remote_client_id_ = "";
   std::unordered_set<int> pressed_keyboard_keys_;
   std::mutex pressed_keyboard_keys_mutex_;
-  SDL_Event last_mouse_event;
-  SDL_AudioStream* output_stream_;
+  SDL_Event last_mouse_event{};
+  SDL_AudioStream* output_stream_ = nullptr;
   uint32_t STREAM_REFRESH_EVENT = 0;
 #if _WIN32
   std::atomic<bool> pending_windows_service_sas_{false};
@@ -684,8 +684,8 @@ class Render {
   // Map file_id to FileTransferState for global file transfer (props == null)
   std::unordered_map<uint32_t, FileTransferState*> file_id_to_transfer_state_;
   std::shared_mutex file_id_to_transfer_state_mutex_;
-  SDL_AudioDeviceID input_dev_;
-  SDL_AudioDeviceID output_dev_;
+  SDL_AudioDeviceID input_dev_ = 0;
+  SDL_AudioDeviceID output_dev_ = 0;
   ScreenCapturerFactory* screen_capturer_factory_ = nullptr;
   ScreenCapturer* screen_capturer_ = nullptr;
   SpeakerCapturerFactory* speaker_capturer_factory_ = nullptr;
@@ -694,7 +694,7 @@ class Render {
   MouseController* mouse_controller_ = nullptr;
   KeyboardCapturer* keyboard_capturer_ = nullptr;
   std::vector<DisplayInfo> display_info_list_;
-  uint64_t last_frame_time_;
+  uint64_t last_frame_time_ = 0;
   std::string last_video_frame_stream_id_;
   bool show_new_version_icon_ = false;
   bool show_new_version_icon_in_menu_ = true;

src/gui/render_callback.cpp

@@ -1196,8 +1196,13 @@ void Render::OnReceiveDataBufferCb(const char* data, size_t size,
           remote_action.k.extended);
     } else if (remote_action.type == ControlType::display_id &&
                render->screen_capturer_) {
-      render->selected_display_ = remote_action.d;
-      render->screen_capturer_->SwitchTo(remote_action.d);
+      const int ret = render->screen_capturer_->SwitchTo(remote_action.d);
+      if (ret == 0) {
+        render->selected_display_ = remote_action.d;
+      } else {
+        LOG_WARN("Display switch skipped, invalid display_id={}",
+                 remote_action.d);
+      }
     }
   }
 }

src/screen_capturer/macosx/screen_capturer_sck.cpp

@@ -16,7 +16,11 @@ int ScreenCapturerSck::Init(const int fps, cb_desktop_data cb) {
   }
 
   screen_capturer_sck_impl_ = CreateScreenCapturerSck();
-  screen_capturer_sck_impl_->Init(fps, on_data_);
+  const int ret = screen_capturer_sck_impl_->Init(fps, on_data_);
+  if (ret != 0) {
+    screen_capturer_sck_impl_.reset();
+    return ret;
+  }
 
   return 0;
 }
@@ -29,8 +33,11 @@ int ScreenCapturerSck::Destroy() {
 }
 
 int ScreenCapturerSck::Start(bool show_cursor) {
-  screen_capturer_sck_impl_->Start(show_cursor);
-  return 0;
+  if (!screen_capturer_sck_impl_) {
+    return -1;
+  }
+
+  return screen_capturer_sck_impl_->Start(show_cursor);
 }
 
 int ScreenCapturerSck::Stop() {
@@ -80,4 +87,4 @@ std::vector<DisplayInfo> ScreenCapturerSck::GetDisplayInfoList() {
 void ScreenCapturerSck::OnFrame() {}
 
 void ScreenCapturerSck::CleanUp() {}
-}  // namespace crossdesk
+}  // namespace crossdesk

src/screen_capturer/macosx/screen_capturer_sck_impl.mm

@@ -16,7 +16,12 @@
 #include <IOKit/graphics/IOGraphicsLib.h>
 #include <IOSurface/IOSurface.h>
 #include <ScreenCaptureKit/ScreenCaptureKit.h>
+#include <algorithm>
 #include <atomic>
+#include <cctype>
+#include <cstring>
+#include <limits>
+#include <map>
 #include <mutex>
 #include <vector>
 #include "display_info.h"
@@ -28,6 +33,15 @@ class ScreenCapturerSckImpl;
 
 static const int kFullDesktopScreenId = -1;
 
+static std::string NSErrorToString(NSError *error) {
+  if (!error) {
+    return "";
+  }
+
+  const char *description = [error.localizedDescription UTF8String];
+  return description ? description : "";
+}
+
 // The ScreenCaptureKit API was available in macOS 12.3, but full-screen capture
 // was reported to be broken before macOS 13 - see http://crbug.com/40234870.
 // Also, the `SCContentFilter` fields `contentRect` and `pointPixelScale` were
@@ -78,6 +92,7 @@ class API_AVAILABLE(macos(14.0)) ScreenCapturerSckImpl : public ScreenCapturer {
   std::map<CGDirectDisplayID, int> display_id_map_reverse_;
   std::map<CGDirectDisplayID, std::string> display_id_name_map_;
   unsigned char *nv12_frame_ = nullptr;
+  size_t nv12_frame_size_ = 0;
   int width_ = 0;
   int height_ = 0;
   int fps_ = 60;
@@ -100,7 +115,7 @@ class API_AVAILABLE(macos(14.0)) ScreenCapturerSckImpl : public ScreenCapturer {
   // Helper object to receive Objective-C callbacks from ScreenCaptureKit and call into this C++
   // object. The helper may outlive this C++ instance, if a completion-handler is passed to
   // ScreenCaptureKit APIs and the C++ object is deleted before the handler executes.
-  SckHelper *__strong helper_;
+  SckHelper *__strong helper_ = nil;
   // Callback for returning captured frames, or errors, to the caller. Only used on the caller's
   // thread.
   cb_desktop_data _on_data = nullptr;
@@ -110,7 +125,7 @@ class API_AVAILABLE(macos(14.0)) ScreenCapturerSckImpl : public ScreenCapturer {
   // Guards some variables that may be accessed on different threads.
   std::mutex lock_;
   // Provides captured desktop frames.
-  SCStream *__strong stream_;
+  SCStream *__strong stream_ = nil;
   // Currently selected display, or 0 if the full desktop is selected. This capturer does not
   // support full-desktop capture, and will fall back to the first display.
   CGDirectDisplayID current_display_ = 0;
@@ -182,6 +197,19 @@ ScreenCapturerSckImpl::ScreenCapturerSckImpl() {
 }
 
 ScreenCapturerSckImpl::~ScreenCapturerSckImpl() {
+  SckHelper *helper_to_release = nil;
+  {
+    std::lock_guard<std::mutex> lock(lock_);
+    if (stream_) {
+      [stream_ stopCaptureWithCompletionHandler:nil];
+      stream_ = nil;
+    }
+    _on_data = nullptr;
+    helper_to_release = helper_;
+    helper_ = nil;
+  }
+  [helper_to_release releaseCapturer];
+
   display_info_list_.clear();
   display_id_map_.clear();
   display_id_map_reverse_.clear();
@@ -190,15 +218,22 @@ ScreenCapturerSckImpl::~ScreenCapturerSckImpl() {
   if (nv12_frame_) {
     delete[] nv12_frame_;
     nv12_frame_ = nullptr;
+    nv12_frame_size_ = 0;
   }
-
-  [stream_ stopCaptureWithCompletionHandler:nil];
-  [helper_ releaseCapturer];
 }
 
 int ScreenCapturerSckImpl::Init(const int fps, cb_desktop_data cb) {
+  if (!cb) {
+    LOG_ERROR("Screen capturer callback is null");
+    return -1;
+  }
+
   _on_data = cb;
-  fps_ = fps;
+  fps_ = fps > 0 ? fps : 60;
+  display_info_list_.clear();
+  display_id_map_.clear();
+  display_id_map_reverse_.clear();
+  display_id_name_map_.clear();
 
   if (@available(macOS 10.15, *)) {
     bool has_permission = CGPreflightScreenCaptureAccess();
@@ -216,8 +251,7 @@ int ScreenCapturerSckImpl::Init(const int fps, cb_desktop_data cb) {
       getShareableContentWithCompletionHandler:^(SCShareableContent *result, NSError *error) {
         if (error) {
           capture_error = error;
-          LOG_ERROR("Failed to get shareable content: {}",
-                    std::string([error.localizedDescription UTF8String]));
+          LOG_ERROR("Failed to get shareable content: {}", NSErrorToString(error));
         } else {
           content = result;
         }
@@ -227,7 +261,7 @@ int ScreenCapturerSckImpl::Init(const int fps, cb_desktop_data cb) {
 
   if (capture_error || !content || content.displays.count == 0) {
     LOG_ERROR("Failed to get display info, error: {}",
-              std::string([capture_error.localizedDescription UTF8String]));
+              NSErrorToString(capture_error));
     return -1;
   }
 
@@ -284,51 +318,58 @@ int ScreenCapturerSckImpl::Start(bool show_cursor) {
 }
 
 int ScreenCapturerSckImpl::SwitchTo(int monitor_index) {
-  if (stream_) {
-    [stream_ stopCaptureWithCompletionHandler:^(NSError *error) {
-      std::lock_guard<std::mutex> lock(lock_);
-      stream_ = nil;
-      current_display_ = display_id_map_[monitor_index];
-      StartOrReconfigureCapturer();
-    }];
-  } else {
-    current_display_ = display_id_map_[monitor_index];
-    StartOrReconfigureCapturer();
+  auto display_it = display_id_map_.find(monitor_index);
+  if (display_it == display_id_map_.end()) {
+    LOG_WARN("SwitchTo skipped, invalid monitor_index={}, displays={}",
+             monitor_index, display_id_map_.size());
+    return -1;
   }
+
+  const CGDirectDisplayID target_display = display_it->second;
+  {
+    std::lock_guard<std::mutex> lock(lock_);
+    current_display_ = target_display;
+  }
+  StartOrReconfigureCapturer();
   return 0;
 }
 
 int ScreenCapturerSckImpl::ResetToInitialMonitor() {
   int target = initial_monitor_index_;
   if (display_info_list_.empty()) return -1;
-  CGDirectDisplayID target_display = display_id_map_[target];
-  if (current_display_ == target_display) return 0;
-  if (stream_) {
-    [stream_ stopCaptureWithCompletionHandler:^(NSError *error) {
-      std::lock_guard<std::mutex> lock(lock_);
-      stream_ = nil;
-      current_display_ = target_display;
-      StartOrReconfigureCapturer();
-    }];
-  } else {
-    current_display_ = target_display;
-    StartOrReconfigureCapturer();
+  auto display_it = display_id_map_.find(target);
+  if (display_it == display_id_map_.end()) {
+    LOG_WARN("ResetToInitialMonitor skipped, invalid monitor_index={}", target);
+    return -1;
   }
+
+  CGDirectDisplayID target_display = display_it->second;
+  if (current_display_ == target_display) return 0;
+  {
+    std::lock_guard<std::mutex> lock(lock_);
+    current_display_ = target_display;
+  }
+  StartOrReconfigureCapturer();
   return 0;
 }
 
 int ScreenCapturerSckImpl::Destroy() {
-  std::lock_guard<std::mutex> lock(lock_);
-  if (stream_) {
-    LOG_INFO("Destroying stream");
-    [stream_ stopCaptureWithCompletionHandler:nil];
-    stream_ = nil;
+  SckHelper *helper_to_release = nil;
+  {
+    std::lock_guard<std::mutex> lock(lock_);
+    if (stream_) {
+      LOG_INFO("Destroying stream");
+      [stream_ stopCaptureWithCompletionHandler:nil];
+      stream_ = nil;
+    }
+    current_display_ = 0;
+    permanent_error_ = false;
+    _on_data = nullptr;
+    helper_to_release = helper_;
+    helper_ = nil;
   }
-  current_display_ = 0;
-  permanent_error_ = false;
-  _on_data = nullptr;
-  [helper_ releaseCapturer];
-  helper_ = nil;
+
+  [helper_to_release releaseCapturer];
 
   return 0;
 }
@@ -416,7 +457,7 @@ void ScreenCapturerSckImpl::OnShareableContentCreated(SCShareableContent *conten
 
     // TODO: crbug.com/327458809 - Choose an appropriate sampleHandlerQueue for
     // best performance.
-    NSError *add_stream_output_error;
+    NSError *add_stream_output_error = nil;
     dispatch_queue_t queue = dispatch_queue_create("ScreenCaptureKit.Queue", DISPATCH_QUEUE_SERIAL);
     bool add_stream_output_result = [stream_ addStreamOutput:helper_
                                                         type:SCStreamOutputTypeScreen
@@ -425,7 +466,7 @@ void ScreenCapturerSckImpl::OnShareableContentCreated(SCShareableContent *conten
 
     if (!add_stream_output_result) {
       stream_ = nil;
-      LOG_ERROR("addStreamOutput failed");
+      LOG_ERROR("addStreamOutput failed: {}", NSErrorToString(add_stream_output_error));
       permanent_error_ = true;
       return;
     }
@@ -436,7 +477,7 @@ void ScreenCapturerSckImpl::OnShareableContentCreated(SCShareableContent *conten
         // calls stopCaptureWithCompletionHandler on the stream, which cancels
         // this handler.
         permanent_error_ = true;
-        LOG_ERROR("startCaptureWithCompletionHandler failed");
+        LOG_ERROR("startCaptureWithCompletionHandler failed: {}", NSErrorToString(error));
       } else {
         LOG_INFO("Capture started");
       }
@@ -448,8 +489,18 @@ void ScreenCapturerSckImpl::OnShareableContentCreated(SCShareableContent *conten
 
 void ScreenCapturerSckImpl::OnNewCVPixelBuffer(CVPixelBufferRef pixelBuffer,
                                                CFDictionaryRef attachment) {
+  (void)attachment;
+  if (!pixelBuffer) {
+    return;
+  }
+
   size_t width = CVPixelBufferGetWidth(pixelBuffer);
   size_t height = CVPixelBufferGetHeight(pixelBuffer);
+  if (width == 0 || height == 0 || CVPixelBufferGetPlaneCount(pixelBuffer) < 2) {
+    LOG_ERROR("Invalid CVPixelBuffer: width={}, height={}, planes={}", width, height,
+              CVPixelBufferGetPlaneCount(pixelBuffer));
+    return;
+  }
 
   CVReturn status = CVPixelBufferLockBaseAddress(pixelBuffer, kCVPixelBufferLock_ReadOnly);
   if (status != kCVReturnSuccess) {
@@ -458,18 +509,37 @@ void ScreenCapturerSckImpl::OnNewCVPixelBuffer(CVPixelBufferRef pixelBuffer,
   }
 
   size_t required_size = width * height * 3 / 2;
-  if (!nv12_frame_ || (width_ * height_ * 3 / 2 < required_size)) {
+  if (required_size > static_cast<size_t>((std::numeric_limits<int>::max)())) {
+    LOG_ERROR("Captured frame is too large: {} bytes", required_size);
+    CVPixelBufferUnlockBaseAddress(pixelBuffer, kCVPixelBufferLock_ReadOnly);
+    return;
+  }
+
+  std::lock_guard<std::mutex> lock(lock_);
+  if (!_on_data) {
+    CVPixelBufferUnlockBaseAddress(pixelBuffer, kCVPixelBufferLock_ReadOnly);
+    return;
+  }
+
+  if (!nv12_frame_ || nv12_frame_size_ < required_size) {
     delete[] nv12_frame_;
     nv12_frame_ = new unsigned char[required_size];
-    width_ = width;
-    height_ = height;
+    nv12_frame_size_ = required_size;
   }
+  width_ = static_cast<int>(width);
+  height_ = static_cast<int>(height);
 
   void *base_y = CVPixelBufferGetBaseAddressOfPlane(pixelBuffer, 0);
   size_t stride_y = CVPixelBufferGetBytesPerRowOfPlane(pixelBuffer, 0);
 
   void *base_uv = CVPixelBufferGetBaseAddressOfPlane(pixelBuffer, 1);
   size_t stride_uv = CVPixelBufferGetBytesPerRowOfPlane(pixelBuffer, 1);
+  if (!base_y || !base_uv || stride_y < width || stride_uv < width) {
+    LOG_ERROR("Invalid CVPixelBuffer planes: base_y={}, base_uv={}, stride_y={}, stride_uv={}",
+              base_y != nullptr, base_uv != nullptr, stride_y, stride_uv);
+    CVPixelBufferUnlockBaseAddress(pixelBuffer, kCVPixelBufferLock_ReadOnly);
+    return;
+  }
 
   unsigned char *dst_y = nv12_frame_;
   for (size_t row = 0; row < height; ++row) {
@@ -481,7 +551,8 @@ void ScreenCapturerSckImpl::OnNewCVPixelBuffer(CVPixelBufferRef pixelBuffer,
     memcpy(dst_uv + row * width, static_cast<unsigned char *>(base_uv) + row * stride_uv, width);
   }
 
-  _on_data(nv12_frame_, width * height * 3 / 2, width, height,
+  _on_data(nv12_frame_, static_cast<int>(required_size), static_cast<int>(width),
+           static_cast<int>(height),
            display_id_name_map_[current_display_].c_str());
 
   CVPixelBufferUnlockBaseAddress(pixelBuffer, kCVPixelBufferLock_ReadOnly);
@@ -503,10 +574,14 @@ void ScreenCapturerSckImpl::StartOrReconfigureCapturer() {
   }
 
   SckHelper *local_helper = helper_;
+  if (!local_helper) {
+    LOG_ERROR("Cannot reconfigure capturer: helper is null");
+    return;
+  }
+
   auto handler = ^(SCShareableContent *content, NSError *error) {
     if (error) {
-      LOG_ERROR("getShareableContent failed: {}",
-                std::string([error.localizedDescription UTF8String]));
+      LOG_ERROR("getShareableContent failed: {}", NSErrorToString(error));
       [local_helper onShareableContentCreated:nil];
       return;
     }
@@ -576,4 +651,4 @@ void ScreenCapturerSckImpl::StartOrReconfigureCapturer() {
 
 std::unique_ptr<ScreenCapturer> ScreenCapturerSck::CreateScreenCapturerSck() {
   return std::make_unique<ScreenCapturerSckImpl>();
-}
+}

src/speaker_capturer/macosx/speaker_capturer_macosx.mm

@@ -9,6 +9,17 @@ namespace crossdesk {
 class SpeakerCapturerMacosx;
 }
 
+namespace {
+std::string NSErrorToString(NSError* error) {
+  if (!error) {
+    return "";
+  }
+
+  const char* description = [error.localizedDescription UTF8String];
+  return description ? description : "";
+}
+}  // namespace
+
 @interface SpeakerCaptureDelegate : NSObject <SCStreamDelegate, SCStreamOutput>
 @property(nonatomic, assign) crossdesk::SpeakerCapturerMacosx* owner;
 - (instancetype)initWithOwner:(crossdesk::SpeakerCapturerMacosx*)owner;
@@ -28,15 +39,36 @@ class SpeakerCapturerMacosx;
                    ofType:(SCStreamOutputType)type {
   if (type != SCStreamOutputTypeAudio) return;
 
+  crossdesk::SpeakerCapturerMacosx* owner = _owner;
+  if (!owner || !owner->cb_) {
+    return;
+  }
+
   CMBlockBufferRef blockBuffer = CMSampleBufferGetDataBuffer(sampleBuffer);
+  if (!blockBuffer) {
+    return;
+  }
+
   size_t length = CMBlockBufferGetDataLength(blockBuffer);
   char* dataPtr = NULL;
-  CMBlockBufferGetDataPointer(blockBuffer, 0, NULL, NULL, &dataPtr);
+  OSStatus dataStatus =
+      CMBlockBufferGetDataPointer(blockBuffer, 0, NULL, NULL, &dataPtr);
+  if (dataStatus != noErr || dataPtr == nullptr || length == 0) {
+    return;
+  }
+
   CMAudioFormatDescriptionRef formatDesc = CMSampleBufferGetFormatDescription(sampleBuffer);
+  if (!formatDesc) {
+    return;
+  }
+
   const AudioStreamBasicDescription* asbd =
       CMAudioFormatDescriptionGetStreamBasicDescription(formatDesc);
+  if (!asbd || asbd->mChannelsPerFrame == 0) {
+    return;
+  }
 
-  if (_owner->cb_ && dataPtr && length > 0 && asbd) {
+  if (owner->cb_) {
     std::vector<short> out_pcm16;
     if (asbd->mFormatFlags & kAudioFormatFlagIsFloat) {
       int channels = asbd->mChannelsPerFrame;
@@ -86,7 +118,10 @@ class SpeakerCapturerMacosx;
     size_t total_bytes = out_pcm16.size() * sizeof(short);
     unsigned char* p = (unsigned char*)out_pcm16.data();
     for (size_t offset = 0; offset + frame_bytes <= total_bytes; offset += frame_bytes) {
-      _owner->cb_(p + offset, frame_bytes, "audio");
+      if (!owner->cb_) {
+        return;
+      }
+      owner->cb_(p + offset, frame_bytes, "audio");
     }
   }
 }
@@ -155,7 +190,7 @@ int SpeakerCapturerMacosx::Init(speaker_data_cb cb) {
 
   if (error || !impl_->content) {
     LOG_ERROR("Failed to get shareable content: {}",
-              std::string([error.localizedDescription UTF8String]));
+              NSErrorToString(error));
     return -1;
   }
 
@@ -209,7 +244,7 @@ int SpeakerCapturerMacosx::Start() {
                                      error:&addOutputError];
   if (!ok || addOutputError) {
     LOG_ERROR("addStreamOutput error: {}",
-              std::string([addOutputError.localizedDescription UTF8String]));
+              NSErrorToString(addOutputError));
     impl_->stream = nil;
     impl_->delegate = nil;
     return -1;
@@ -220,7 +255,7 @@ int SpeakerCapturerMacosx::Start() {
   [impl_->stream startCaptureWithCompletionHandler:^(NSError* _Nullable error) {
     if (error) {
       LOG_ERROR("startCaptureWithCompletionHandler error: {}",
-                std::string([error.localizedDescription UTF8String]));
+                NSErrorToString(error));
       ret = -1;
     }
     dispatch_semaphore_signal(semaStart);
@@ -238,13 +273,14 @@ int SpeakerCapturerMacosx::Stop() {
   [impl_->stream stopCaptureWithCompletionHandler:^(NSError* error) {
     if (error) {
       LOG_ERROR("stopCaptureWithCompletionHandler error: {}",
-                std::string([error.localizedDescription UTF8String]));
+                NSErrorToString(error));
     }
     dispatch_semaphore_signal(sema);
   }];
   dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER);
 
   impl_->stream = nil;
+  impl_->delegate.owner = nullptr;
   impl_->delegate = nil;
 
   return 0;
@@ -269,4 +305,4 @@ int SpeakerCapturerMacosx::Destroy() {
 int SpeakerCapturerMacosx::Pause() { return 0; }
 
 int SpeakerCapturerMacosx::Resume() { return Start(); }
-}  // namespace crossdesk
+}  // namespace crossdesk