Skip to content

Commit 4228a90

Browse files
committed
fix: add bounds checking for file operations and string lengths
- add assertions for freopen operations in PreInit - limit string length with kStrlenMaxLength in GFBuffer - add bounds checking for file read operations - simplify error handling in ReadFileGFBuffer
1 parent a6d70b1 commit 4228a90

3 files changed

Lines changed: 17 additions & 18 deletions

File tree

src/Initialize.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ void PreInit(const GFCxtWPtr &p_ctx) {
6969
const auto console = app->property("GFShowConsoleOnWindows").toBool();
7070

7171
if (console && AllocConsole()) {
72-
freopen("CONOUT$", "w", stdout);
73-
freopen("CONOUT$", "w", stderr);
74-
freopen("CONIN$", "r", stdin);
72+
Q_ASSERT(freopen("CONOUT$", "w", stdout) != nullptr);
73+
Q_ASSERT(freopen("CONOUT$", "w", stderr) != nullptr);
74+
Q_ASSERT(freopen("CONIN$", "r", stdin) != nullptr);
7575
setvbuf(stdout, NULL, _IONBF, 0);
7676

7777
qInstallMessageHandler([](QtMsgType type, const QMessageLogContext &context,

src/core/model/GFBuffer.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,14 @@
3131
#include <openssl/crypto.h>
3232
#include <openssl/evp.h>
3333

34+
#include <cstddef>
3435
#include <cstring>
3536

3637
namespace GpgFrontend {
3738

39+
// 16MB
40+
constexpr size_t kStrlenMaxLength = static_cast<const size_t>(16 * 1024 * 1024);
41+
3842
struct GFBuffer::Impl {
3943
void* sec_ptr_ = nullptr;
4044
size_t sec_size_ = 0;
@@ -96,8 +100,8 @@ GFBuffer::GFBuffer(const QString& str) {
96100
}
97101

98102
GFBuffer::GFBuffer(const char* str)
99-
: impl_(SecureCreateSharedObject<Impl>((str != nullptr) ? std::strlen(str)
100-
: 0)) {
103+
: impl_(SecureCreateSharedObject<Impl>(
104+
(str != nullptr) ? strnlen(str, kStrlenMaxLength) : 0)) {
101105
if ((str != nullptr) && impl_->sec_size_ > 0) {
102106
std::memcpy(impl_->sec_ptr_, str, impl_->sec_size_);
103107
}
@@ -214,12 +218,12 @@ auto GFBuffer::ConvertToQString() const -> QString {
214218
}
215219

216220
auto GFBuffer::operator==(const char* str) const -> bool {
217-
return Size() == strlen(str) &&
221+
return Size() == strnlen(str, kStrlenMaxLength) &&
218222
(Size() == 0 || std::memcmp(Data(), str, Size()) == 0);
219223
}
220224

221225
auto GFBuffer::operator!=(const char* str) const -> bool {
222-
return Size() == strlen(str) &&
226+
return Size() == strnlen(str, kStrlenMaxLength) &&
223227
(Size() == 0 || std::memcmp(Data(), str, Size()) != 0);
224228
}
225229

src/core/utils/IOUtils.cpp

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,11 @@ auto GetFileHashOpenSSL(const QString& file_path, const EVP_MD* md_type)
4848
}
4949

5050
GpgFrontend::GFBuffer buffer(GpgFrontend::kSecBufferSize);
51+
const auto buffer_size = static_cast<qsizetype>(buffer.Size());
5152
while (!file.atEnd()) {
52-
qint64 n = file.read(buffer.Data(), static_cast<qsizetype>(buffer.Size()));
53-
if (n <= 0) break;
53+
auto n = file.read(buffer.Data(), buffer_size);
54+
if (n <= 0 || n > buffer_size) break;
55+
5456
if (EVP_DigestUpdate(ctx, buffer.Data(), n) != 1) {
5557
EVP_MD_CTX_free(ctx);
5658
return {};
@@ -100,18 +102,11 @@ auto ReadFileGFBuffer(const QString& file_name) -> std::tuple<bool, GFBuffer> {
100102
}
101103

102104
auto file_size = file.size();
103-
if (file_size <= 0) {
104-
file.close();
105-
return {false, GFBuffer()};
106-
}
105+
if (file_size <= 0) return {false, GFBuffer()};
107106

108107
GFBuffer buf(static_cast<size_t>(file_size));
109108
auto n = file.read(buf.Data(), file_size);
110-
file.close();
111-
112-
if (n != file_size) {
113-
return {false, GFBuffer()};
114-
}
109+
if (n <= 0 || n != file_size) return {false, GFBuffer()};
115110

116111
return {true, std::move(buf)};
117112
}

0 commit comments

Comments
 (0)