From fd4116862dc9db260624437c9b54e28c93647b7e Mon Sep 17 00:00:00 2001 From: wangwenjun Date: Wed, 11 May 2022 14:01:41 +0800 Subject: [PATCH 1/4] Null pointer risk. Signed-off-by: wangwenjun --- .../dscreen_sink_handler_fuzzer.cpp | 3 +++ .../dscreen_source_handler_fuzzer.cpp | 3 +++ .../src/screen_client_window_adapter.cpp | 12 ++++++++++++ .../dscreenmgr/include/screen_manager_adapter.h | 2 +- .../sourceservice/dscreenmgr/src/dscreen.cpp | 12 ++++++++++++ .../dscreenmgr/src/dscreen_manager.cpp | 16 ++++++++++++++++ .../dscreenmgr/src/screen_manager_adapter.cpp | 12 ++++++++---- .../src/image_sink_processor.cpp | 10 ++++++++++ .../screensinktrans/src/screen_sink_trans.cpp | 4 ++++ 9 files changed, 69 insertions(+), 5 deletions(-) diff --git a/interfaces/innerkits/native_cpp/test/fuzztest/dscreensinkhandler_fuzzer/dscreen_sink_handler_fuzzer.cpp b/interfaces/innerkits/native_cpp/test/fuzztest/dscreensinkhandler_fuzzer/dscreen_sink_handler_fuzzer.cpp index 66853e0d..895b3125 100644 --- a/interfaces/innerkits/native_cpp/test/fuzztest/dscreensinkhandler_fuzzer/dscreen_sink_handler_fuzzer.cpp +++ b/interfaces/innerkits/native_cpp/test/fuzztest/dscreensinkhandler_fuzzer/dscreen_sink_handler_fuzzer.cpp @@ -37,6 +37,9 @@ void DscreenSinkHandlerFuzzTest(const uint8_t* data, size_t size) sptr samgr = SystemAbilityManagerClient::GetInstance().GetSystemAbilityManager(); + if (samgr == nullptr) { + return; + } sptr remoteObject = samgr->GetSystemAbility(DISTRIBUTED_HARDWARE_SCREEN_SINK_SA_ID); wptr remote (remoteObject); DScreenSinkHandler::GetInstance().InitSink(params); diff --git a/interfaces/innerkits/native_cpp/test/fuzztest/dscreensourcehandler_fuzzer/dscreen_source_handler_fuzzer.cpp b/interfaces/innerkits/native_cpp/test/fuzztest/dscreensourcehandler_fuzzer/dscreen_source_handler_fuzzer.cpp index 9bfc6fef..f1495fdc 100644 --- a/interfaces/innerkits/native_cpp/test/fuzztest/dscreensourcehandler_fuzzer/dscreen_source_handler_fuzzer.cpp +++ b/interfaces/innerkits/native_cpp/test/fuzztest/dscreensourcehandler_fuzzer/dscreen_source_handler_fuzzer.cpp @@ -47,6 +47,9 @@ void DscreenSourceHandlerFuzzTest(const uint8_t* data, size_t size) sptr samgr = SystemAbilityManagerClient::GetInstance().GetSystemAbilityManager(); + if (samgr == nullptr) { + return; + } sptr remoteObject = samgr->GetSystemAbility(DISTRIBUTED_HARDWARE_SCREEN_SOURCE_SA_ID); wptr remote (remoteObject); diff --git a/services/screenclient/src/screen_client_window_adapter.cpp b/services/screenclient/src/screen_client_window_adapter.cpp index 9e3eb78c..f2d8cd1d 100644 --- a/services/screenclient/src/screen_client_window_adapter.cpp +++ b/services/screenclient/src/screen_client_window_adapter.cpp @@ -197,18 +197,30 @@ int32_t ScreenClientWindowAdapter::RemoveWindow(int32_t windowId) void ScreenClientInputEventListener::OnInputEvent(std::shared_ptr pointerEvent) const { DHLOGI("onInputEvent, pointerEvent"); + if (pointerEvent == nullptr) { + DHLOGI("pointerEvent is nullptr."); + return; + } pointerEvent->MarkProcessed(); } void ScreenClientInputEventListener::OnInputEvent(std::shared_ptr keyEvent) const { DHLOGI("onInputEvent, keyEvent"); + if (keyEvent == nullptr) { + DHLOGI("keyEvent is nullptr."); + return; + } keyEvent->MarkProcessed(); } void ScreenClientInputEventListener::OnInputEvent(std::shared_ptr axisEvent) const { DHLOGI("onInputEvent, axisEvent"); + if (axisEvent == nullptr) { + DHLOGI("onInputEvent, pointerEvent"); + return; + } axisEvent->MarkProcessed(); } } // namespace DistributedHardware diff --git a/services/screenservice/sourceservice/dscreenmgr/include/screen_manager_adapter.h b/services/screenservice/sourceservice/dscreenmgr/include/screen_manager_adapter.h index da25606f..c795dcf6 100644 --- a/services/screenservice/sourceservice/dscreenmgr/include/screen_manager_adapter.h +++ b/services/screenservice/sourceservice/dscreenmgr/include/screen_manager_adapter.h @@ -44,7 +44,7 @@ private: ScreenMgrAdapter() = default; ~ScreenMgrAdapter(); - bool listenerRegistered = false; + bool listenerRegistered_ = false; std::map screenIdMap_; }; } // namespace DistributedHardware diff --git a/services/screenservice/sourceservice/dscreenmgr/src/dscreen.cpp b/services/screenservice/sourceservice/dscreenmgr/src/dscreen.cpp index 5e43589a..c7220ff0 100644 --- a/services/screenservice/sourceservice/dscreenmgr/src/dscreen.cpp +++ b/services/screenservice/sourceservice/dscreenmgr/src/dscreen.cpp @@ -181,6 +181,10 @@ void DScreen::HandleTask(const std::shared_ptr &task) void DScreen::HandleEnable(const std::string ¶m, const std::string &taskId) { + if (dscreenCallback_ == nullptr) { + DHLOGE("dscreenCallback_ is nullptr."); + return; + } DHLOGI("HandleEnable, devId: %s, dhId: %s", GetAnonyString(devId_).c_str(), GetAnonyString(dhId_).c_str()); if (curState_ == ENABLED || curState_ == ENABLING || curState_ == CONNECTING || curState_ == CONNECTED) { @@ -248,6 +252,10 @@ int32_t DScreen::NegotiateCodecType(const std::string &remoteCodecInfoStr) std::vector> caps = codecList->GetVideoEncoderCaps(); for (const auto &cap : caps) { std::shared_ptr codecInfo = cap->GetCodecInfo(); + if (codecInfo == nullptr) { + DHLOGE("codecInfo is nullptr."); + return ERR_DH_SCREEN_SA_DSCREEN_NEGOTIATE_CODEC_FAIL; + } localCodecArray.push_back(codecInfo->GetName()); } @@ -279,6 +287,10 @@ void DScreen::HandleDisable(const std::string &taskId) { DHLOGI("HandleDisable, devId: %s, dhId: %s", GetAnonyString(devId_).c_str(), GetAnonyString(dhId_).c_str()); SetState(DISABLING); + if (dscreenCallback_ == nullptr) { + DHLOGE("dscreenCallback_ is nullptr."); + return; + } int32_t ret = ScreenMgrAdapter::GetInstance().RemoveVirtualScreen(screenId_); if (ret != DH_SUCCESS) { DHLOGE("remove virtual screen failed."); diff --git a/services/screenservice/sourceservice/dscreenmgr/src/dscreen_manager.cpp b/services/screenservice/sourceservice/dscreenmgr/src/dscreen_manager.cpp index 49812b88..6be7e437 100644 --- a/services/screenservice/sourceservice/dscreenmgr/src/dscreen_manager.cpp +++ b/services/screenservice/sourceservice/dscreenmgr/src/dscreen_manager.cpp @@ -157,6 +157,10 @@ void DScreenManager::HandleScreenChange(const std::shared_ptr &changedS void DScreenCallback::OnRegResult(const std::shared_ptr &dScreen, const std::string &reqId, int32_t status, const std::string &data) { + if (dScreen == nullptr) { + DHLOGE("dScreen is nullptr."); + return; + } DHLOGI("DScreenCallback::OnRegResult, devId: %s, dhId: %s, reqId: %s", GetAnonyString(dScreen->GetDevId()).c_str(), GetAnonyString(dScreen->GetDHId()).c_str(), reqId.c_str()); DScreenManager::GetInstance().OnRegResult(dScreen, reqId, status, data); @@ -165,6 +169,10 @@ void DScreenCallback::OnRegResult(const std::shared_ptr &dScreen, void DScreenCallback::OnUnregResult(const std::shared_ptr &dScreen, const std::string &reqId, int32_t status, const std::string &data) { + if (dScreen == nullptr) { + DHLOGE("dScreen is nullptr."); + return; + } DHLOGI("DScreenCallback::OnUnregResult, devId: %s, dhId: %s, reqId: %s", GetAnonyString(dScreen->GetDevId()).c_str(), GetAnonyString(dScreen->GetDHId()).c_str(), reqId.c_str()); DScreenManager::GetInstance().OnUnregResult(dScreen, reqId, status, data); @@ -173,6 +181,10 @@ void DScreenCallback::OnUnregResult(const std::shared_ptr &dScreen, void DScreenManager::OnRegResult(const std::shared_ptr &dScreen, const std::string &reqId, int32_t status, const std::string &data) { + if (dScreen == nullptr) { + DHLOGE("dScreen is nullptr."); + return; + } DHLOGI("DScreenManager::OnRegResult, devId: %s, dhId: %s, reqId: %s", GetAnonyString(dScreen->GetDevId()).c_str(), GetAnonyString(dScreen->GetDHId()).c_str(), reqId.c_str()); if (!dScreenSourceCallbackProxy_) { @@ -185,6 +197,10 @@ void DScreenManager::OnRegResult(const std::shared_ptr &dScreen, void DScreenManager::OnUnregResult(const std::shared_ptr &dScreen, const std::string &reqId, int32_t status, const std::string &data) { + if (dScreen == nullptr) { + DHLOGE("dScreen is nullptr."); + return; + } DHLOGI("DScreenManager::OnUnregResult, devId: %s, dhId: %s, reqId: %s", GetAnonyString(dScreen->GetDevId()).c_str(), GetAnonyString(dScreen->GetDHId()).c_str(), reqId.c_str()); if (!dScreenSourceCallbackProxy_) { diff --git a/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp b/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp index 6e6cb06a..ec3ecde4 100644 --- a/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp +++ b/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp @@ -39,6 +39,10 @@ uint64_t ScreenMgrAdapter::CreateVirtualScreen(const std::string &devId, const s { DHLOGI("CreateVirtualScreen, width: %u, height: %u", videoParam->GetScreenWidth(), videoParam->GetScreenHeight()); + if (videoParam == nullptr) { + DHLOGI("videoParam is nullptr."); + return SCREEN_ID_INVALID; + } std::string screenName = DSCREEN_PREFIX + SEPERATOR + devId + SEPERATOR + dhId; auto iter = screenIdMap_.find(screenName); if (iter != screenIdMap_.end()) { @@ -69,7 +73,7 @@ uint64_t ScreenMgrAdapter::CreateVirtualScreen(const std::string &devId, const s int32_t ScreenMgrAdapter::RegisterScreenGroupListener(sptr listener) { DHLOGI("RegisterScreenGroupListener"); - if (listenerRegistered) { + if (listenerRegistered_) { DHLOGI("already registered listener."); return DH_SUCCESS; } @@ -78,14 +82,14 @@ int32_t ScreenMgrAdapter::RegisterScreenGroupListener(sptr listener) { DHLOGI("UnregisterScreenGroupListener"); - if (!listenerRegistered) { + if (!listenerRegistered_) { DHLOGI("listener already unregistered."); return DH_SUCCESS; } @@ -94,7 +98,7 @@ int32_t ScreenMgrAdapter::UnregisterScreenGroupListener(sptr &surface) { DHLOGI("%s: SetImageSurface.", LOG_TAG); + if (surface == nullptr) { + DHLOGE("%s: surface is null.", LOG_TAG); + return ERR_DH_SCREEN_TRANS_NULL_VALUE; + } + if (imageDecoder_ == nullptr) { DHLOGE("%s: Decoder is null.", LOG_TAG); return ERR_DH_SCREEN_TRANS_NULL_VALUE; @@ -114,6 +119,11 @@ int32_t ImageSinkProcessor::SetImageSurface(sptr &surface) int32_t ImageSinkProcessor::ProcessImage(const std::shared_ptr &data) { DHLOGI("%s: ProcessImage.", LOG_TAG); + if (data == nullptr) { + DHLOGE("%s: data is null.", LOG_TAG); + return ERR_DH_SCREEN_TRANS_NULL_VALUE; + } + if (imageDecoder_ == nullptr) { DHLOGE("%s: Decoder is null.", LOG_TAG); return ERR_DH_SCREEN_TRANS_NULL_VALUE; diff --git a/services/screentransport/screensinktrans/src/screen_sink_trans.cpp b/services/screentransport/screensinktrans/src/screen_sink_trans.cpp index 6de90dc2..7f561060 100644 --- a/services/screentransport/screensinktrans/src/screen_sink_trans.cpp +++ b/services/screentransport/screensinktrans/src/screen_sink_trans.cpp @@ -257,6 +257,10 @@ int32_t ScreenSinkTrans::RegisterProcessorListener(const VideoParam &localParam, DHLOGE("%s: Channel listener to null.", LOG_TAG); return ERR_DH_SCREEN_TRANS_NULL_VALUE; } + if (imageProcessor_ == nullptr) { + DHLOGE("%s: imageProcessor_ is null.", LOG_TAG); + return ERR_DH_SCREEN_TRANS_NULL_VALUE; + } int32_t ret = imageProcessor_->ConfigureImageProcessor(localParam, remoteParam, listener); if (ret != DH_SUCCESS) { -- Gitee From fc7a9e4ae1048c12de9c7be8b3eef8450615f451 Mon Sep 17 00:00:00 2001 From: wangwenjun Date: Wed, 11 May 2022 14:35:58 +0800 Subject: [PATCH 2/4] Null pointer risk. Signed-off-by: wangwenjun --- screenhandler/src/dscreen_handler.cpp | 4 ++++ .../screensinkprocessor/src/image_sink_processor.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/screenhandler/src/dscreen_handler.cpp b/screenhandler/src/dscreen_handler.cpp index 1e51bd30..f50e8cd4 100644 --- a/screenhandler/src/dscreen_handler.cpp +++ b/screenhandler/src/dscreen_handler.cpp @@ -161,6 +161,10 @@ std::string DScreenHandler::QueryCodecInfo() // query codec info std::shared_ptr codecList = Media::AVCodecListFactory::CreateAVCodecList(); + if (codecList == nullptr) { + DHLOGE("codecList is nullptr."); + return codecInfoStr_; + } std::vector> caps = codecList->GetVideoEncoderCaps(); json codecTypeArray = json::array(); diff --git a/services/screentransport/screensinkprocessor/src/image_sink_processor.cpp b/services/screentransport/screensinkprocessor/src/image_sink_processor.cpp index eb41c25f..ff6674c3 100644 --- a/services/screentransport/screensinkprocessor/src/image_sink_processor.cpp +++ b/services/screentransport/screensinkprocessor/src/image_sink_processor.cpp @@ -94,11 +94,11 @@ int32_t ImageSinkProcessor::StopImageProcessor() return DH_SUCCESS; } -int32_t ImageSinkProcessor::SetImageSurface(sptr &surface) +int32_t ImageSinkProcessor::SetImageSurface(sptr &imageSurface) { DHLOGI("%s: SetImageSurface.", LOG_TAG); - if (surface == nullptr) { - DHLOGE("%s: surface is null.", LOG_TAG); + if (imageSurface == nullptr) { + DHLOGE("%s: imageSurface is null.", LOG_TAG); return ERR_DH_SCREEN_TRANS_NULL_VALUE; } @@ -107,7 +107,7 @@ int32_t ImageSinkProcessor::SetImageSurface(sptr &surface) return ERR_DH_SCREEN_TRANS_NULL_VALUE; } - int32_t ret = imageDecoder_->SetOutputSurface(surface); + int32_t ret = imageDecoder_->SetOutputSurface(imageSurface); if (ret != DH_SUCCESS) { DHLOGE("%s: SetOutputSurface failed ret:%d.", LOG_TAG, ret); return ret; -- Gitee From 71b9f912092f84711f4c24bbae1d55c9075d26ab Mon Sep 17 00:00:00 2001 From: wangwenjun Date: Mon, 16 May 2022 14:51:53 +0800 Subject: [PATCH 3/4] G.FUD.05 problem. Signed-off-by: wangwenjun --- .../sourceservice/dscreenmgr/src/screen_manager_adapter.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp b/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp index ec3ecde4..c781c8ec 100644 --- a/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp +++ b/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp @@ -54,7 +54,6 @@ uint64_t ScreenMgrAdapter::CreateVirtualScreen(const std::string &devId, const s } screenIdMap_.erase(screenName); } - Rosen::VirtualScreenOption option = { screenName, videoParam->GetScreenWidth(), @@ -64,7 +63,6 @@ uint64_t ScreenMgrAdapter::CreateVirtualScreen(const std::string &devId, const s DEFAULT_SCREEN_FLAGS, false }; - uint64_t screenId = Rosen::ScreenManager::GetInstance().CreateVirtualScreen(option); screenIdMap_.emplace(screenName, screenId); return screenId; -- Gitee From b84e5f186c7f8022ce8bcbc150d881f9e3c5f178 Mon Sep 17 00:00:00 2001 From: wangwenjun Date: Mon, 16 May 2022 16:31:48 +0800 Subject: [PATCH 4/4] crash risk. Signed-off-by: wangwenjun --- .../sourceservice/dscreenmgr/src/screen_manager_adapter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp b/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp index c781c8ec..dd39a4f1 100644 --- a/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp +++ b/services/screenservice/sourceservice/dscreenmgr/src/screen_manager_adapter.cpp @@ -37,12 +37,12 @@ ScreenMgrAdapter::~ScreenMgrAdapter() uint64_t ScreenMgrAdapter::CreateVirtualScreen(const std::string &devId, const std::string &dhId, const std::shared_ptr &videoParam) { - DHLOGI("CreateVirtualScreen, width: %u, height: %u", videoParam->GetScreenWidth(), - videoParam->GetScreenHeight()); if (videoParam == nullptr) { DHLOGI("videoParam is nullptr."); return SCREEN_ID_INVALID; } + DHLOGI("CreateVirtualScreen, width: %u, height: %u", videoParam->GetScreenWidth(), + videoParam->GetScreenHeight()); std::string screenName = DSCREEN_PREFIX + SEPERATOR + devId + SEPERATOR + dhId; auto iter = screenIdMap_.find(screenName); if (iter != screenIdMap_.end()) { -- Gitee