Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions packages/camera/camera_windows/windows/camera_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,28 @@ std::optional<std::string> GetFilePathForPicture() {
ComHeapPtr<wchar_t> known_folder_path;
HRESULT hr = SHGetKnownFolderPath(FOLDERID_Pictures, KF_FLAG_CREATE, nullptr,
&known_folder_path);
if (FAILED(hr)) {
return std::nullopt;

std::wstring wpath;

if (SUCCEEDED(hr)) {
wpath = std::wstring(known_folder_path);
} else {
// Fallback to temp folder
wchar_t tempPath[MAX_PATH];
DWORD len = GetTempPathW(MAX_PATH, tempPath);
if (len == 0 || len > MAX_PATH) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The condition len > MAX_PATH is incorrect and could lead to a buffer over-read. For WinAPI functions like GetTempPathW, if the returned length is equal to the buffer size (MAX_PATH in this case), the written string is not guaranteed to be null-terminated. Constructing a std::wstring from a non-null-terminated C-style string results in undefined behavior as it will read past the end of the buffer. The check should be len >= MAX_PATH to correctly handle this edge case.

Suggested change
if (len == 0 || len > MAX_PATH) {
if (len == 0 || len >= MAX_PATH) {

return std::nullopt;
}
wpath = std::wstring(tempPath);
}
Comment on lines +98 to 108

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This change introduces new logic, including a fallback path, but doesn't include corresponding tests. The repository style guide states that 'Code should be tested'. To ensure the correctness and robustness of this new functionality, please add unit tests that cover both the successful retrieval of the Pictures folder and the fallback to the temporary directory. This might require some refactoring to allow for mocking of the Windows API calls.

References
  1. Code should be tested. Changes to plugin packages, which include code written in C, C++, Java, Kotlin, Objective-C, or Swift, should have appropriate tests. (link)


std::string path = Utf8FromUtf16(std::wstring(known_folder_path));
if (!wpath.empty() && wpath.back() != L'\\' && wpath.back() != L'/') {
wpath.push_back(L'\\');
}

std::string path = Utf8FromUtf16(wpath);

return path + "\\" + "PhotoCapture_" + GetCurrentTimeString() + "." +
return path + "PhotoCapture_" + GetCurrentTimeString() + "." +
kPictureCaptureExtension;
}

Expand Down