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

diff --git a/cdr/cdr_pgsql.c b/cdr/cdr_pgsql.c
index aea56691bd134c46c0211840e4b81119241f9a65..4430e51053aa9e1e5a004fde1e708f264d69bc3e 100644 (file)
--- a/cdr/cdr_pgsql.c
+++ b/cdr/cdr_pgsql.c
@@ -217,6 +217,7 @@ static int pgsql_log(struct ast_cdr *cdr)
        struct ast_tm tm;
        char *pgerror;
        PGresult *result;
+       int res = -1;
 
        ast_mutex_lock(&pgsql_lock);
 
@@ -246,13 +247,14 @@ static int pgsql_log(struct ast_cdr *cdr)
        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 @@ static int pgsql_log(struct ast_cdr *cdr)
                                        }
                                /* 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 @@ static int pgsql_log(struct ast_cdr *cdr)
                                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 @@ static int pgsql_log(struct ast_cdr *cdr)
                                        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 4ab3878fdcd3b78afa2be942c045db6ff0ab5142..a79bfab748d3df1cc73464cf033f06f60169ff20 100644 (file)
--- a/cel/cel_pgsql.c
+++ b/cel/cel_pgsql.c
@@ -325,6 +325,7 @@ static void pgsql_log(struct ast_event *event)
                                                        char *tmpbuf = ast_realloc(escapebuf, required_size);
 
                                                        if (!tmpbuf) {
+                                                               AST_RWLIST_UNLOCK(&psql_columns);
                                                                goto ast_log_cleanup;
                                                        }
 
@@ -385,8 +386,6 @@ static void pgsql_log(struct ast_event *event)
                                        ast_log(LOG_ERROR, "Reason: %s\n", pgerror);
                                }
                        }
-                       PQclear(result);
-                       goto ast_log_cleanup;
                }
                PQclear(result);