From 59024932caa82f5ec8711a7749279563cccbbd4c Mon Sep 17 00:00:00 2001 From: Ana Bobillo Date: Thu, 22 Oct 2026 20:56:23 +0200 Subject: [PATCH v1 3/3] libpq: Add a test of tracing malformed protocol messages From: Ana Bobillo libpq_trace_regress includes fe-trace.c, as test-oauth-curl.c includes oauth-curl.c, and traces one message given on its command line, in a buffer of exactly the message's size, so that AddressSanitizer or a similar tool catches any read past the end of the message. t/007_trace.pl runs it over well-formed messages and over messages whose contents don't fit in their length, and checks what is printed. --- src/interfaces/libpq/meson.build | 1 + src/interfaces/libpq/t/007_trace.pl | 137 ++++++++++++++++++ src/interfaces/libpq/test/.gitignore | 1 + src/interfaces/libpq/test/Makefile | 7 +- .../libpq/test/libpq_trace_regress.c | 91 ++++++++++++ src/interfaces/libpq/test/meson.build | 21 +++ 6 files changed, 256 insertions(+), 2 deletions(-) diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build index b0ae72167a1..39a36471445 100644 --- a/src/interfaces/libpq/meson.build +++ b/src/interfaces/libpq/meson.build @@ -161,6 +161,7 @@ tests += { 't/004_load_balance_dns.pl', 't/005_negotiate_encryption.pl', 't/006_service.pl', + 't/007_trace.pl', ], 'env': { 'with_ssl': ssl_library, diff --git a/src/interfaces/libpq/t/007_trace.pl b/src/interfaces/libpq/t/007_trace.pl new file mode 100644 index 00000000000..e218813b3d8 --- /dev/null +++ b/src/interfaces/libpq/t/007_trace.pl @@ -0,0 +1,137 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test libpq's protocol message tracing with messages whose contents don't +# fit in their length. The tracer must stop at the end of the message, and +# report it, instead of reading past it. +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Utils; +use Test::More; +use IPC::Run; + +# Each test is the direction (F or B), the message type, its contents in hex, +# and the expected trace output. libpq_trace_regress fills in the length. +my @tests = ( + # Well-formed messages + [ 'B', 'D', '0002 00000003 616263 ffffffff', qq{B\t17\tDataRow\t 2 3 'abc' -1} ], + [ 'B', 'N', '53 4e4f5449434500 00', qq{B\t13\tNoticeResponse\t S "NOTICE" \\x00} ], + [ 'B', 'd', '68656c6c6f0a', qq{B\t10\tCopyData\t 'hello\\x0a'} ], + [ 'B', 'K', '00000001 0203', qq{B\t10\tBackendKeyData\t 1 '\\x02\\x03'} ], + [ 'F', 'C', '53 666f6f00', qq{F\t9\tClose\t S "foo"} ], + [ + 'B', 'R', '0000000a 534352414d2d5348412d32353600 00', + qq{B\t23\tAuthenticationSASL\t "SCRAM-SHA-256" ""} + ], + [ + 'B', 'v', '00030000 00000001 5f707100', + qq{B\t16\tNegotiateProtocolVersion\t 196608 1 "_pq"} + ], + [ 'B', 'W', '00 0001 0000', qq{B\t9\tCopyBothResponse\t \\x00 1 0} ], + + # A negative field length other than -1 is a NULL too, as libpq reads it + [ 'B', 'D', '0002 fffffffe 00000001 62', qq{B\t15\tDataRow\t 2 -2 1 'b'} ], + + # Contents left over at the end are still reported + [ + 'B', 'Z', '4900', + qq{B\t6\tReadyForQuery\t I\nmismatched message length: consumed 5, expected 6} + ], + + # Fields that run past the end of the message + [ + 'B', 'D', '0001 000003e8 616263', + qq{B\t13\tDataRow\t 1 1000 'abc'\nmismatched message length: contents exceed length 13} + ], + [ + 'B', 'D', '0001 7fffffff 61', + qq{B\t11\tDataRow\t 1 2147483647 'a'\nmismatched message length: contents exceed length 11} + ], + [ + 'B', 'V', '00000004 6162', + qq{B\t10\tFunctionCallResponse\t 4 'ab'\nmismatched message length: contents exceed length 10} + ], + [ + 'B', 'V', 'fffffffb', + qq{B\t8\tFunctionCallResponse\t -5 ''\nmismatched message length: contents exceed length 8} + ], + + # A frontend message type is printed as such, even coming from the server + [ + 'B', 'B', '00 00 0001 0000 0001 000000ff 41', + qq{B\t17\tBind\t "" "" 1 0 1 255 'A'\nmismatched message length: contents exceed length 17} + ], + + # Counts of more fields than the message holds + [ + 'B', 'D', '0003 00000001 61', + qq{B\t11\tDataRow\t 3 1 'a'\nmismatched message length: contents exceed length 11} + ], + [ + 'B', 'T', '0001 6100 00000000 0000', + qq{B\t14\tRowDescription\t 1 "a" 0 0\nmismatched message length: contents exceed length 14} + ], + [ + 'B', 't', '0002 00000017', + qq{B\t10\tParameterDescription\t 2 23\nmismatched message length: contents exceed length 10} + ], + [ + 'B', 'G', '00 0003 0000', + qq{B\t9\tCopyInResponse\t \\x00 3 0\nmismatched message length: contents exceed length 9} + ], + [ + 'B', 'v', '00030000 7fffffff 5f707100', + qq{B\t16\tNegotiateProtocolVersion\t 196608 2147483647 "_pq"\nmismatched message length: contents exceed length 16} + ], + [ + 'B', 'W', '00 0003 0000', + qq{B\t9\tCopyBothResponse\t \\x00 3 0\nmismatched message length: contents exceed length 9} + ], + [ + 'B', 'W', '00 0001 00', + qq{B\t8\tCopyBothResponse\t \\x00 1\nmismatched message length: contents exceed length 8} + ], + + # Strings and lists without their terminating zero byte + [ + 'B', 'C', '53454c454354', + qq{B\t10\tCommandComplete\t "SELECT"\nmismatched message length: contents exceed length 10} + ], + [ + 'B', 'S', '6100 62', + qq{B\t7\tParameterStatus\t "a" "b"\nmismatched message length: contents exceed length 7} + ], + [ + 'B', 'A', '00000001 6300', + qq{B\t10\tNotificationResponse\t 1 "c"\nmismatched message length: contents exceed length 10} + ], + [ + 'B', 'E', '53 4552524f5200', + qq{B\t11\tErrorResponse\t S "ERROR"\nmismatched message length: contents exceed length 11} + ], + [ + 'B', 'R', '0000000a 534352414d2d5348412d32353600', + qq{B\t22\tAuthenticationSASL\t "SCRAM-SHA-256"\nmismatched message length: contents exceed length 22} + ], + [ + 'B', 'R', '', + qq{B\t4\t\nmismatched message length: contents exceed length 4} + ]); + +foreach my $test (@tests) +{ + my ($direction, $type, $data, $expected) = @$test; + my $name = "pqTraceOutputMessage: $direction '$type' $data"; + + my ($stdout, $stderr); + my $result = IPC::Run::run [ 'libpq_trace_regress', $direction, $type, $data ], + '>' => \$stdout, + '2>' => \$stderr; + chomp($stdout); + + ok($result, "$name: exit status"); + is($stdout, $expected, "$name: trace"); + is($stderr, '', "$name: no errors"); +} + +done_testing(); diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore index 6ba78adb678..04a6b3144e0 100644 --- a/src/interfaces/libpq/test/.gitignore +++ b/src/interfaces/libpq/test/.gitignore @@ -1,2 +1,3 @@ /libpq_testclient +/libpq_trace_regress /libpq_uri_regress diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile index 4e17ec15141..6abb82f7b7e 100644 --- a/src/interfaces/libpq/test/Makefile +++ b/src/interfaces/libpq/test/Makefile @@ -11,14 +11,17 @@ ifeq ($(PORTNAME), win32) LDFLAGS += -lws2_32 endif -override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) +override CPPFLAGS := -I$(libpq_srcdir) -I$(top_builddir)/src/port -I$(top_srcdir)/src/port $(CPPFLAGS) LDFLAGS_INTERNAL += $(libpq_pgport) -PROGS = libpq_testclient libpq_uri_regress +PROGS = libpq_testclient libpq_trace_regress libpq_uri_regress all: $(PROGS) $(PROGS): $(WIN32RES) +# libpq_trace_regress.c includes fe-trace.c +libpq_trace_regress.o: $(libpq_srcdir)/fe-trace.c + clean distclean: rm -f $(PROGS) *.o diff --git a/src/interfaces/libpq/test/libpq_trace_regress.c b/src/interfaces/libpq/test/libpq_trace_regress.c new file mode 100644 index 00000000000..1f5999bb6d6 --- /dev/null +++ b/src/interfaces/libpq/test/libpq_trace_regress.c @@ -0,0 +1,91 @@ +/* + * libpq_trace_regress.c + * A test program for libpq's protocol message tracing + * + * This #includes fe-trace.c, so that it can call pqTraceOutputMessage() + * directly. It traces one message, built from its arguments, in a buffer of + * exactly its size, so that a memory checker such as AddressSanitizer catches + * any read past its end. Without one, the test still checks the output. + * + * Usage: libpq_trace_regress F|B TYPE HEXDATA + * + * F or B says whether the message goes to the server or comes from it, TYPE + * is its type byte and HEXDATA its contents in hex (spaces are ignored). The + * length word is filled in to match the contents. + * + * Copyright (c) 2026, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/interfaces/libpq/test/libpq_trace_regress.c + */ + +#include "fe-trace.c" + +static int +hexval(char c) +{ + if (c >= '0' && c <= '9') + return c - '0'; + if (c >= 'a' && c <= 'f') + return c - 'a' + 10; + if (c >= 'A' && c <= 'F') + return c - 'A' + 10; + return -1; +} + +int +main(int argc, char *argv[]) +{ + PGconn conn; + const char *hex; + char *message; + int len = 0; + uint32 n32; + + if (argc != 4 || (strcmp(argv[1], "F") != 0 && strcmp(argv[1], "B") != 0) || + strlen(argv[2]) != 1) + { + fprintf(stderr, "usage: %s F|B TYPE HEXDATA\n", argv[0]); + return 1; + } + + /* Type byte, length word, then the contents */ + message = malloc(1 + 4 + strlen(argv[3]) / 2); + if (message == NULL) + { + fprintf(stderr, "out of memory\n"); + return 1; + } + for (hex = argv[3]; *hex; hex++) + { + if (*hex == ' ') + continue; + if (hexval(hex[0]) < 0 || hex[1] == '\0' || hexval(hex[1]) < 0) + { + fprintf(stderr, "invalid hex data \"%s\"\n", argv[3]); + return 1; + } + message[1 + 4 + len++] = hexval(hex[0]) * 16 + hexval(hex[1]); + hex++; + } + message[0] = argv[2][0]; + n32 = pg_hton32(4 + len); + memcpy(message + 1, &n32, 4); + + /* Make the buffer exactly as long as the message */ + message = realloc(message, 1 + 4 + len); + if (message == NULL) + { + fprintf(stderr, "out of memory\n"); + return 1; + } + + memset(&conn, 0, sizeof(conn)); + conn.Pfdebug = stdout; + conn.traceFlags = PQTRACE_SUPPRESS_TIMESTAMPS; + + pqTraceOutputMessage(&conn, message, argv[1][0] == 'F'); + + free(message); + return 0; +} diff --git a/src/interfaces/libpq/test/meson.build b/src/interfaces/libpq/test/meson.build index e203486615c..52d27d5a457 100644 --- a/src/interfaces/libpq/test/meson.build +++ b/src/interfaces/libpq/test/meson.build @@ -39,4 +39,25 @@ libpq_test_deps += executable('libpq_testclient', } ) +libpq_trace_regress_sources = files( + 'libpq_trace_regress.c', +) + +if host_system == 'windows' + libpq_trace_regress_sources += rc_bin_gen.process(win32ver_rc, extra_args: [ + '--NAME', 'libpq_trace_regress', + '--FILEDESC', 'libpq test program',]) +endif + +# This includes fe-trace.c, so it needs libpq's internal headers (and the +# headers they include) but not the library +libpq_test_deps += executable('libpq_trace_regress', + libpq_trace_regress_sources, + include_directories: [libpq_inc], + dependencies: [frontend_no_fe_utils_code, ssl, gssapi], + kwargs: default_bin_args + { + 'install': false, + } +) + testprep_targets += libpq_test_deps