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
pull/7/head
Sean Bright 8 years ago
parent 7898aad02d
commit c537f99488

@ -202,6 +202,7 @@ static int pgsql_log(struct ast_cdr *cdr)
struct ast_tm tm; struct ast_tm tm;
char *pgerror; char *pgerror;
PGresult *result; PGresult *result;
int res = -1;
ast_mutex_lock(&pgsql_lock); ast_mutex_lock(&pgsql_lock);
@ -231,13 +232,14 @@ static int pgsql_log(struct ast_cdr *cdr)
if (connected) { if (connected) {
struct columns *cur; struct columns *cur;
struct ast_str *sql = ast_str_create(maxsize), *sql2 = ast_str_create(maxsize2); 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;
char *separator = ""; char *separator = "";
size_t bufsize = 513;
if (!sql || !sql2) { escapebuf = ast_malloc(bufsize);
ast_free(sql); if (!escapebuf || !sql || !sql2) {
ast_free(sql2); goto ast_log_cleanup;
return -1;
} }
ast_str_set(&sql, 0, "INSERT INTO %s (", table); ast_str_set(&sql, 0, "INSERT INTO %s (", table);
@ -358,10 +360,28 @@ static int pgsql_log(struct ast_cdr *cdr)
} }
/* XXX Might want to handle dates, times, and other misc fields here XXX */ /* XXX Might want to handle dates, times, and other misc fields here XXX */
} else { } 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); PQescapeStringConn(conn, escapebuf, value, strlen(value), NULL);
else } else {
escapebuf[0] = '\0'; escapebuf[0] = '\0';
}
LENGTHEN_BUF2(strlen(escapebuf) + 3); LENGTHEN_BUF2(strlen(escapebuf) + 3);
ast_str_append(&sql2, 0, "%s'%s'", separator, escapebuf); ast_str_append(&sql2, 0, "%s'%s'", separator, escapebuf);
} }
@ -395,10 +415,7 @@ static int pgsql_log(struct ast_cdr *cdr)
PQfinish(conn); PQfinish(conn);
conn = NULL; conn = NULL;
connected = 0; connected = 0;
ast_mutex_unlock(&pgsql_lock); goto ast_log_cleanup;
ast_free(sql);
ast_free(sql2);
return -1;
} }
} }
result = PQexec(conn, ast_str_buffer(sql)); result = PQexec(conn, ast_str_buffer(sql));
@ -419,23 +436,17 @@ static int pgsql_log(struct ast_cdr *cdr)
pgerror = PQresultErrorMessage(result); pgerror = PQresultErrorMessage(result);
ast_log(LOG_ERROR, "HARD ERROR! Attempted reconnection failed. DROPPING CALL RECORD!\n"); ast_log(LOG_ERROR, "HARD ERROR! Attempted reconnection failed. DROPPING CALL RECORD!\n");
ast_log(LOG_ERROR, "Reason: %s\n", pgerror); ast_log(LOG_ERROR, "Reason: %s\n", pgerror);
} else { } else {
/* Second try worked out ok */ /* Second try worked out ok */
totalrecords++; totalrecords++;
records++; records++;
ast_mutex_unlock(&pgsql_lock); res = 0;
PQclear(result);
return 0;
} }
} }
ast_mutex_unlock(&pgsql_lock);
PQclear(result);
ast_free(sql);
ast_free(sql2);
return -1;
} else { } else {
totalrecords++; totalrecords++;
records++; records++;
res = 0;
} }
PQclear(result); PQclear(result);
@ -447,11 +458,14 @@ static int pgsql_log(struct ast_cdr *cdr)
maxsize2 = ast_str_strlen(sql2); maxsize2 = ast_str_strlen(sql2);
} }
ast_log_cleanup:
ast_free(escapebuf);
ast_free(sql); ast_free(sql);
ast_free(sql2); ast_free(sql2);
} }
ast_mutex_unlock(&pgsql_lock); ast_mutex_unlock(&pgsql_lock);
return 0; return res;
} }
/* This function should be called without holding the pgsql_columns lock */ /* This function should be called without holding the pgsql_columns lock */

@ -320,6 +320,7 @@ static void pgsql_log(struct ast_event *event)
char *tmpbuf = ast_realloc(escapebuf, required_size); char *tmpbuf = ast_realloc(escapebuf, required_size);
if (!tmpbuf) { if (!tmpbuf) {
AST_RWLIST_UNLOCK(&psql_columns);
goto ast_log_cleanup; goto ast_log_cleanup;
} }
@ -380,8 +381,6 @@ static void pgsql_log(struct ast_event *event)
ast_log(LOG_ERROR, "Reason: %s\n", pgerror); ast_log(LOG_ERROR, "Reason: %s\n", pgerror);
} }
} }
PQclear(result);
goto ast_log_cleanup;
} }
PQclear(result); PQclear(result);

Loading…
Cancel
Save