From 07ca0504c9b3a009ea5180944ef5a7d5b5d16452 Mon Sep 17 00:00:00 2001
From: Dmitry Ivanov <ivadmi5@gmail.com>
Date: Thu, 10 Aug 2023 16:02:42 +0300
Subject: [PATCH] fix: pgproto should not pass null-terminated queries to
Sbroad
---
pgproto/src/postgres/port.c | 19 ++++++++++++++++++
pgproto/src/postgres/port.h | 11 +++++++++++
pgproto/src/postgres/postgres.c | 32 +++++++------------------------
pgproto/test/CMakeLists.txt | 4 +++-
pgproto/test/simple_query_test.py | 17 ++++++----------
5 files changed, 46 insertions(+), 37 deletions(-)
diff --git a/pgproto/src/postgres/port.c b/pgproto/src/postgres/port.c
index 5777ba93f3..03a8ac57c7 100644
--- a/pgproto/src/postgres/port.c
+++ b/pgproto/src/postgres/port.c
@@ -77,6 +77,25 @@ pg_port_close(struct pg_port *port)
free(port->user);
}
+char *
+pg_read_cstr(struct pg_port *port, size_t *len)
+{
+ uint32_t packet_len;
+ if (pg_read_uint32(port, &packet_len) < 0)
+ return NULL;
+
+ if (packet_len < sizeof(uint32_t) + 1)
+ return NULL;
+
+ size_t cstr_len = packet_len - sizeof(uint32_t);
+ char *query = pg_read_bytes(port, cstr_len);
+ if (query == NULL || query[cstr_len - 1] != '\0')
+ return NULL;
+
+ *len = cstr_len - 1;
+ return query;
+}
+
void *
pg_read_bytes(struct pg_port *port, size_t size)
{
diff --git a/pgproto/src/postgres/port.h b/pgproto/src/postgres/port.h
index 965ae0cef8..a37f2565e9 100644
--- a/pgproto/src/postgres/port.h
+++ b/pgproto/src/postgres/port.h
@@ -77,6 +77,17 @@ pg_port_create(struct pg_port *port, struct iostream *io);
void
pg_port_close(struct pg_port *port);
+/**
+ * Read a null-terminated string to the port buffer, set size to its length
+ * excluding the terminator and return a pointer to the data read.
+ *
+ * @retval not-NULL on success.
+ * @retval NULL on EOF or error,
+ * check pg_port::state to understand what happened.
+ */
+char *
+pg_read_cstr(struct pg_port *port, size_t *size);
+
/**
* Read size bytes to the port buffer and return a pointer to the data read.
*
diff --git a/pgproto/src/postgres/postgres.c b/pgproto/src/postgres/postgres.c
index 49da853f29..1aeca7db5f 100644
--- a/pgproto/src/postgres/postgres.c
+++ b/pgproto/src/postgres/postgres.c
@@ -1,8 +1,8 @@
#include <module.h>
#include <msgpuck.h>
#include <inttypes.h>
-#include <strings.h> /* for strncasecmp */
-#include <ctype.h> /* for isspace */
+#include <strings.h>
+#include <ctype.h>
#include "postgres.h"
#include "report.h"
@@ -12,7 +12,6 @@
#include "attributes.h"
#include "tarantool/trivia/util.h"
-
/**
* Format of ReadyForQuery message.
* ReadyForQuery informs the frontend that it can safely send a new command.
@@ -56,26 +55,6 @@ struct query_message {
const char *query;
};
-/** Read query string consistin in QueryMessage */
-static const char *
-read_query_string(struct pg_port *port, size_t *query_len)
-{
- struct query_message message;
- if (pg_read_uint32(port, &message.len) < 0)
- goto error;
-
- *query_len = message.len - sizeof(message.len);
- message.query = pg_read_bytes(port, *query_len);
- if (message.query == NULL)
- goto error;
-
- return message.query;
-error:
- pg_error(NULL, ERRCODE_INTERNAL_ERROR,
- "failed to read a query message");
- return NULL;
-}
-
#define COMPARE_AND_RETURN_IF_EQUALS(query, tag) \
if (strncasecmp(query, tag, strlen(tag)) == 0) \
return tag
@@ -360,9 +339,12 @@ static int
process_simple_query_impl(struct pg_port *port)
{
size_t query_len;
- const char *query = read_query_string(port, &query_len);
- if (query == NULL)
+ const char *query = pg_read_cstr(port, &query_len);
+ if (query == NULL) {
+ pg_error(port, ERRCODE_INTERNAL_ERROR,
+ "failed to read a query message");
return -1;
+ }
pg_debug("processing query \'%s\'", query);
const char *response = dispatch_query_wrapped(query, query_len);
diff --git a/pgproto/test/CMakeLists.txt b/pgproto/test/CMakeLists.txt
index 20bf908351..c5932d4558 100644
--- a/pgproto/test/CMakeLists.txt
+++ b/pgproto/test/CMakeLists.txt
@@ -1,9 +1,11 @@
get_all_targets(SOURCE_TARGETS ${PROJECT_SOURCE_DIR}/src)
+set(LUA_CPATH "${PROJECT_BINARY_DIR}/src/server/?.so")
+
add_custom_target(test
DEPENDS ${SOURCE_TARGETS}
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
COMMAND ${CMAKE_COMMAND} -E env
"PICODATA_EXECUTABLE=${PICODATA_EXECUTABLE}"
- "LUA_CPATH=${PROJECT_BINARY_DIR}/src/server/?.so"
+ "LUA_CPATH=${LUA_CPATH}"
pytest)
diff --git a/pgproto/test/simple_query_test.py b/pgproto/test/simple_query_test.py
index 110aada8d2..6638208a7a 100644
--- a/pgproto/test/simple_query_test.py
+++ b/pgproto/test/simple_query_test.py
@@ -7,25 +7,20 @@ import time
def start_pg_server(instance, host, service):
start_pg_server_lua_code = f"""
package.cpath="{os.environ['LUA_CPATH']}"
- net_box = require('net.box')
- box.schema.func.create('tcpserver.server_start', {{language = 'C'}})
+
+ box.schema.func.create('tcpserver.server_start', {{ language = 'C' }})
box.schema.user.grant('guest', 'execute', 'function', 'tcpserver.server_start')
- box.cfg{{listen=3301}}
- caller = net_box:new(3301)
- caller:call('tcpserver.server_start', {{ '{host}', '{service}' }})
+ box.func['tcpserver.server_start']:call({{ '{host}', '{service}' }})
"""
instance.eval(start_pg_server_lua_code)
def stop_pg_server(instance):
stop_pg_server_lua_code = f"""
- local net_box = require('net.box')
box.schema.func.create('tcpserver.server_stop', {{language = 'C'}})
box.schema.user.grant('guest', 'execute', 'function', 'tcpserver.server_stop')
- box.cfg{{listen=3301}}
- local caller = net_box:new(3301)
- caller:call('tcpserver.server_stop')
+ box.func['tcpserver.server_stop']:call()
box.schema.func.drop('tcpserver.server_start')
box.schema.func.drop('tcpserver.server_stop')
@@ -69,11 +64,11 @@ def test_simple_flow_session(cluster: Cluster):
i1 = cluster.instances[0]
host = '127.0.0.1'
- service = '35776'
+ service = '5432'
start_pg_server(i1, host, service)
user = 'admin'
- password = '`fANPIOUWEh79p12hdunqwADI`'
+ password = 'password'
i1.eval("box.cfg{auth_type='md5', log_level=7}")
i1.eval(f"box.schema.user.passwd('{user}', '{password}')")
--
GitLab