Skip to content

Commit f95067d

Browse files
Royna2544Royna2544
committed
Fix null pointer dereferences and improve message handling
- Added checks for null pointers in various message handling functions to prevent crashes when accessing optional fields. - Updated the `AuthContext` to handle cases where the sender of a message may not be present. - Modified `MessageExt` to safely access optional fields like `photo`, `sticker`, and `animation`. - Enhanced error logging in `ROMBuildQueryHandler` to handle cases where query messages are null. - Refactored `KernelBuildHandler` to use a new helper function for editing query messages, ensuring null checks are performed. - Adjusted tests to accommodate changes in message handling, ensuring they correctly reference optional fields.
1 parent b7137be commit f95067d

File tree

20 files changed

+264
-188
lines changed

20 files changed

+264
-188
lines changed

‎src/api/AuthContext.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ bool AuthContext::isInList(DatabaseBase::ListType type,
2424
AuthContext::Result AuthContext::isAuthorized(const Message::Ptr& message,
2525
const AccessLevel flags) const {
2626
if (isUnderTimeLimit(message)) {
27-
return isAuthorized(message->from, flags);
27+
if (!message->from) {
28+
// If the message has no sender, we can't authorize it.
29+
return {false, Result::Reason::Unknown};
30+
}
31+
return isAuthorized(*message->from, flags);
2832
} else {
2933
return {false, Result::Reason::MessageTooOld};
3034
}

‎src/api/MessageExt.cpp‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ MessageExt::MessageExt(Message::Ptr message, SplitMessageText how)
1111
if (!_message) {
1212
return;
1313
}
14-
_replyMessage = std::make_shared<MessageExt>(_message->replyToMessage);
14+
_replyMessage = std::make_shared<MessageExt>(
15+
_message->replyToMessage ? *_message->replyToMessage : nullptr);
1516

1617
// Empty message won't need parsing
1718
if (!_message->text) {
@@ -21,26 +22,28 @@ MessageExt::MessageExt(Message::Ptr message, SplitMessageText how)
2122
// Initially, _extra_args is full text
2223
_extra_args = _message->text.value();
2324

24-
// Try to find botcommand entity
25-
const auto botCommandEnt =
26-
std::ranges::find_if(_message->entities, [](const auto& entity) {
27-
return entity->type == TgBot::MessageEntity::Type::BotCommand &&
28-
entity->offset == 0;
29-
});
25+
if (_message->entities) {
26+
// Try to find botcommand entity
27+
const auto botCommandEnt =
28+
std::ranges::find_if(*_message->entities, [](const auto& entity) {
29+
return entity->type == TgBot::MessageEntity::Type::BotCommand &&
30+
entity->offset == 0;
31+
});
3032

31-
// I believe entity must be sent here.
32-
if (botCommandEnt != _message->entities.end() &&
33-
_message->text->front() == '/') {
34-
const auto entry = *botCommandEnt;
35-
// Grab /start@username
36-
_extra_args = _message->text->substr(entry->length);
37-
absl::StripLeadingAsciiWhitespace(&_extra_args);
38-
command.emplace();
39-
std::pair<std::string, std::string> kCommandSplit =
40-
absl::StrSplit(_message->text->substr(1, entry->length), "@");
41-
command->name = kCommandSplit.first;
42-
command->target = kCommandSplit.second;
43-
absl::StripTrailingAsciiWhitespace(&command->target);
33+
// I believe entity must be sent here.
34+
if (botCommandEnt != (*_message->entities).end() &&
35+
_message->text->front() == '/') {
36+
const auto entry = *botCommandEnt;
37+
// Grab /start@username
38+
_extra_args = _message->text->substr(entry->length);
39+
absl::StripLeadingAsciiWhitespace(&_extra_args);
40+
command.emplace();
41+
std::pair<std::string, std::string> kCommandSplit =
42+
absl::StrSplit(_message->text->substr(1, entry->length), "@");
43+
command->name = kCommandSplit.first;
44+
command->target = kCommandSplit.second;
45+
absl::StripTrailingAsciiWhitespace(&command->target);
46+
}
4447
}
4548

4649
if (_extra_args.size() != 0) {

‎src/api/builtin_modules/builder/android/romBuild.cpp‎

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,8 +1160,14 @@ void ROMBuildQueryHandler::handle_clean_directories(const Query& query) {
11601160
bool success = buildService_->cleanDirectory(cleanDirectoryRequest);
11611161
if (!success) {
11621162
LOG(ERROR) << "Failed to clean ROM directory";
1163-
_api->editMessage(query->message, "Failed to clean ROM directory",
1164-
backKeyboard);
1163+
if (query->message) {
1164+
_api->editMessage((*query->message),
1165+
"Failed to clean ROM directory",
1166+
backKeyboard);
1167+
} else {
1168+
LOG(ERROR) << "Query message is null, cannot edit message to "
1169+
"show error";
1170+
}
11651171
return;
11661172
}
11671173
_api->answerCallbackQuery(query->id,
@@ -1176,8 +1182,14 @@ void ROMBuildQueryHandler::handle_clean_directories(const Query& query) {
11761182
bool success = buildService_->cleanDirectory(cleanDirectoryRequest);
11771183
if (!success) {
11781184
LOG(ERROR) << "Failed to clean build directory";
1179-
_api->editMessage(query->message, "Failed to clean build directory",
1180-
backKeyboard);
1185+
if (query->message) {
1186+
_api->editMessage((*query->message),
1187+
"Failed to clean build directory",
1188+
backKeyboard);
1189+
} else {
1190+
LOG(ERROR) << "Query message is null, cannot edit message to "
1191+
"show error";
1192+
}
11811193
return;
11821194
}
11831195
cleanDirectoryRequest.set_directory_type(
@@ -1189,8 +1201,14 @@ void ROMBuildQueryHandler::handle_clean_directories(const Query& query) {
11891201
buildService_->directoryExists(cleanDirectoryRequest, &boolValue);
11901202
if (!checkSuccess) {
11911203
LOG(ERROR) << "Failed to check directory existence";
1192-
_api->editMessage(query->message, "Failed to check directory existence",
1193-
backKeyboard);
1204+
if (query->message) {
1205+
_api->editMessage((*query->message),
1206+
"Failed to check directory existence",
1207+
backKeyboard);
1208+
} else {
1209+
LOG(ERROR)
1210+
<< "Query message is null, cannot edit message to show error";
1211+
}
11941212
return;
11951213
}
11961214

@@ -1209,8 +1227,13 @@ void ROMBuildQueryHandler::handle_clean_directories(const Query& query) {
12091227
!res.ok()) {
12101228
LOG(ERROR) << "Failed to get directory size: "
12111229
<< res.error_message();
1212-
_api->editMessage(query->message, "Failed to get directory size",
1213-
backKeyboard);
1230+
if (query->message) {
1231+
_api->editMessage((*query->message),
1232+
"Failed to get directory size", backKeyboard);
1233+
} else {
1234+
LOG(ERROR) << "Query message is null, cannot edit message to "
1235+
"show error";
1236+
}
12141237
return;
12151238
}
12161239
entry = fmt::format("Current disk space used: {}GB, total: {}GB",
@@ -1221,9 +1244,14 @@ void ROMBuildQueryHandler::handle_clean_directories(const Query& query) {
12211244
buildService_->directoryExists(cleanDirectoryRequest, &boolValue);
12221245
if (!checkSuccess2) {
12231246
LOG(ERROR) << "Failed to check build directory existence";
1224-
_api->editMessage(query->message,
1225-
"Failed to check build directory existence",
1226-
backKeyboard);
1247+
if (query->message) {
1248+
_api->editMessage((*query->message),
1249+
"Failed to check build directory existence",
1250+
backKeyboard);
1251+
} else {
1252+
LOG(ERROR) << "Query message is null, cannot edit message to "
1253+
"show error";
1254+
}
12271255
return;
12281256
}
12291257
builder.addKeyboard({"Clean ROM directory", "clean_rom"});
@@ -1232,7 +1260,13 @@ void ROMBuildQueryHandler::handle_clean_directories(const Query& query) {
12321260
}
12331261
}
12341262
builder.addKeyboard(getButtonOf<Buttons::back>());
1235-
_api->editMessage(query->message, entry, builder.get());
1263+
if (query->message) {
1264+
_api->editMessage((*query->message), entry, builder.get());
1265+
} else {
1266+
LOG(ERROR)
1267+
<< "Query message is null, cannot edit message to show clean "
1268+
"options";
1269+
}
12361270
}
12371271

12381272
void ROMBuildQueryHandler::onCallbackQuery(
@@ -1241,8 +1275,13 @@ void ROMBuildQueryHandler::onCallbackQuery(
12411275
DLOG(INFO) << "No message to handle callback query";
12421276
return;
12431277
}
1244-
if (query->message->chat->id != sentMessage->chat->id ||
1245-
query->message->messageId != sentMessage->messageId) {
1278+
if (!query->message) {
1279+
DLOG(INFO) << "No message in callback query";
1280+
return;
1281+
}
1282+
auto& msg = *query->message;
1283+
if (msg->chat->id != sentMessage->chat->id ||
1284+
msg->messageId != sentMessage->messageId) {
12461285
DLOG(INFO) << "Mismatch on message id";
12471286
return;
12481287
}

0 commit comments

Comments
 (0)