[Asterisk-code-review] cdr pgsql: Fix buffer overflow calling libpq (asterisk[master])

Sean Bright asteriskteam at digium.com
Thu Mar 30 08:17:30 CDT 2017


Sean Bright has uploaded a new change for review. ( https://gerrit.asterisk.org/5367 )

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
1 file changed, 28 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/67/5367/1

diff --git a/cdr/cdr_pgsql.c b/cdr/cdr_pgsql.c
index cbd9e05..8e24280 100644
--- a/cdr/cdr_pgsql.c
+++ b/cdr/cdr_pgsql.c
@@ -231,10 +231,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 *separator = "";
+		char *escapebuf = NULL, *value;
+		size_t bufsize = 513;
 
-		if (!sql || !sql2) {
+		escapebuf = ast_malloc(bufsize);
+		if (!escapebuf || !sql || !sql2) {
+			ast_free(escapebuf);
 			ast_free(sql);
 			ast_free(sql2);
 			return -1;
@@ -358,10 +362,30 @@
 					}
 				/* 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_free(escapebuf);
+								ast_free(sql);
+								ast_free(sql2);
+								return -1;
+							}
+
+							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'", separator, escapebuf);
 				}

-- 
To view, visit https://gerrit.asterisk.org/5367
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaacfa1f1de7cb1e9414d121850d2d8c2888f3f48
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>



More information about the asterisk-code-review mailing list