Skip to content

Commit 0ade6a5

Browse files
author
Marcel Hecko
committed
Fix early_announce crash on DB connect and resource leaks (#287)
- Check fopen() return before fwrite(): when fopen fails (permissions, SELinux, full disk) the NULL FILE* was passed to fwrite(), causing segfault at address 1 inside glibc — the exact crash reported. - Move delete stmt/result before return so they are actually reached; the previous code returned before the deletes, leaking on every query. - Add cleanup of stmt/result in the catch block so an SQL exception does not leak the already-allocated objects. - Guard the get_announce_file function-pointer call in onInvite() against NULL (module not initialised), returning 500 instead of crashing the process. - Add missing <string.h> and <errno.h> includes for strerror(errno). Closes #287
1 parent a1bcde9 commit 0ade6a5

File tree

2 files changed

+28
-10
lines changed

2 files changed

+28
-10
lines changed

apps/early_announce/EarlyAnnounce.cpp

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,34 +94,45 @@ int get_announce_msg(const string &user, const string &domain,
9494
message + "' and language='" + language + "'";
9595
}
9696

97+
sql::Statement *stmt = NULL;
98+
sql::ResultSet *result = NULL;
99+
97100
try {
98-
101+
99102
DBG("Query string <%s>\n", query_string.c_str());
100103

101-
sql::Statement *stmt;
102-
sql::ResultSet *result;
104+
int ret = 1;
103105

104106
stmt = connection->createStatement();
105107
result = stmt->executeQuery(query_string);
106108
if (result->next()) {
107109
FILE *file;
108110
file = fopen((*audio_file).c_str(), "wb");
109-
string s = result->getString("audio");
110-
fwrite(s.data(), 1, s.length(), file);
111-
fclose(file);
112-
return 1;
111+
if (file == NULL) {
112+
ERROR("Cannot open file '%s': %s\n",
113+
audio_file->c_str(), strerror(errno));
114+
*audio_file = "";
115+
ret = 0;
116+
} else {
117+
string s = result->getString("audio");
118+
fwrite(s.data(), 1, s.length(), file);
119+
fclose(file);
120+
}
113121
} else {
114122
*audio_file = "";
115-
return 1;
116123
}
117124

118-
delete stmt;
119125
delete result;
126+
delete stmt;
127+
128+
return ret;
120129

121130
}
122131

123132
catch (sql::SQLException &er) {
124133
ERROR("MySQL query error: %s\n", er.what());
134+
delete result;
135+
delete stmt;
125136
*audio_file = "";
126137
return 0;
127138
}
@@ -332,8 +343,13 @@ AmSession* EarlyAnnounceFactory::onInvite(const AmSipRequest& req, const string&
332343
string language = get_header_keyvalue(iptel_app_param,"Language");
333344
string announce_file = "";
334345

346+
if (get_announce_file == NULL) {
347+
ERROR("early_announce module not initialized\n");
348+
throw AmSession::Exception(500, MOD_NAME " not initialized");
349+
}
350+
335351
get_announce_file(req.user, req.domain, language, &announce_file);
336-
352+
337353
return new EarlyAnnounceDialog(announce_file);
338354
}
339355

apps/early_announce/EarlyAnnounce.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ using std::string;
3939
#include <cppconn/resultset.h>
4040
#include <cppconn/statement.h>
4141
#include <stdio.h>
42+
#include <string.h>
43+
#include <errno.h>
4244
#include <string>
4345
#endif
4446

0 commit comments

Comments
 (0)