[asterisk-commits] cdr pgsql: Fix buffer overflow calling libpq (asterisk[13])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Mar 31 07:05:27 CDT 2017
Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5365 )
Change subject: cdr_pgsql: Fix buffer overflow calling libpq
......................................................................
cdr_pgsql: Fix buffer overflow calling libpq
Implement the same buffer size checking done in cel_pgsql.
ASTERISK-26896 #close
Reported by: twisted
Change-Id: Iaacfa1f1de7cb1e9414d121850d2d8c2888f3f48
---
M cdr/cdr_pgsql.c
M cel/cel_pgsql.c
2 files changed, 37 insertions(+), 23 deletions(-)
Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, approved
diff --git a/cdr/cdr_pgsql.c b/cdr/cdr_pgsql.c
index aea5669..4430e51 100644
--- a/cdr/cdr_pgsql.c
+++ b/cdr/cdr_pgsql.c
@@ -217,6 +217,7 @@
struct ast_tm tm;
char *pgerror;
PGresult *result;
+ int res = -1;
ast_mutex_lock(&pgsql_lock);
@@ -246,13 +247,14 @@
if (connected) {
struct columns *cur;
struct ast_str *sql = ast_str_create(maxsize), *sql2 = ast_str_create(maxsize2);
- char buf[257], escapebuf[513], *value;
+ char buf[257];
+ char *escapebuf = NULL, *value;
int first = 1;
+ size_t bufsize = 513;
- if (!sql || !sql2) {
- ast_free(sql);
- ast_free(sql2);
- return -1;
+ escapebuf = ast_malloc(bufsize);
+ if (!escapebuf || !sql || !sql2) {
+ goto ast_log_cleanup;
}
ast_str_set(&sql, 0, "INSERT INTO %s (", table);
@@ -373,10 +375,28 @@
}
/* XXX Might want to handle dates, times, and other misc fields here XXX */
} else {
- if (value)
+ if (value) {
+ size_t required_size = strlen(value) * 2 + 1;
+
+ /* If our argument size exceeds our buffer, grow it,
+ * as PQescapeStringConn() expects the buffer to be
+ * adequitely sized and does *NOT* do size checking.
+ */
+ if (required_size > bufsize) {
+ char *tmpbuf = ast_realloc(escapebuf, required_size);
+
+ if (!tmpbuf) {
+ AST_RWLIST_UNLOCK(&psql_columns);
+ goto ast_log_cleanup;
+ }
+
+ escapebuf = tmpbuf;
+ bufsize = required_size;
+ }
PQescapeStringConn(conn, escapebuf, value, strlen(value), NULL);
- else
+ } else {
escapebuf[0] = '\0';
+ }
LENGTHEN_BUF2(strlen(escapebuf) + 3);
ast_str_append(&sql2, 0, "%s'%s'", first ? "" : ",", escapebuf);
}
@@ -410,10 +430,7 @@
PQfinish(conn);
conn = NULL;
connected = 0;
- ast_mutex_unlock(&pgsql_lock);
- ast_free(sql);
- ast_free(sql2);
- return -1;
+ goto ast_log_cleanup;
}
}
result = PQexec(conn, ast_str_buffer(sql));
@@ -434,30 +451,28 @@
pgerror = PQresultErrorMessage(result);
ast_log(LOG_ERROR, "HARD ERROR! Attempted reconnection failed. DROPPING CALL RECORD!\n");
ast_log(LOG_ERROR, "Reason: %s\n", pgerror);
- } else {
+ } else {
/* Second try worked out ok */
totalrecords++;
records++;
- ast_mutex_unlock(&pgsql_lock);
- PQclear(result);
- return 0;
+ res = 0;
}
}
- ast_mutex_unlock(&pgsql_lock);
- PQclear(result);
- ast_free(sql);
- ast_free(sql2);
- return -1;
} else {
totalrecords++;
records++;
+ res = 0;
}
PQclear(result);
+
+ast_log_cleanup:
+ ast_free(escapebuf);
ast_free(sql);
ast_free(sql2);
}
+
ast_mutex_unlock(&pgsql_lock);
- return 0;
+ return res;
}
/* This function should be called without holding the pgsql_columns lock */
diff --git a/cel/cel_pgsql.c b/cel/cel_pgsql.c
index 4ab3878..a79bfab 100644
--- a/cel/cel_pgsql.c
+++ b/cel/cel_pgsql.c
@@ -325,6 +325,7 @@
char *tmpbuf = ast_realloc(escapebuf, required_size);
if (!tmpbuf) {
+ AST_RWLIST_UNLOCK(&psql_columns);
goto ast_log_cleanup;
}
@@ -385,8 +386,6 @@
ast_log(LOG_ERROR, "Reason: %s\n", pgerror);
}
}
- PQclear(result);
- goto ast_log_cleanup;
}
PQclear(result);
--
To view, visit https://gerrit.asterisk.org/5365
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaacfa1f1de7cb1e9414d121850d2d8c2888f3f48
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-commits
mailing list