From 800f183a1efd1354f3564273e01eabfd379ce452 Mon Sep 17 00:00:00 2001 From: Dhanraj E Date: Mon, 19 Oct 2026 17:22:18 +0530 Subject: [PATCH] Fixed stack buffer overflow in GetServerLiteral for TLS VAR requests GetServerLiteral() zeroed and formatted CF_BUFSIZE bytes into the sendbuffer it was handed, but BusyWithNewProtocol() allocates that buffer as CF_BUFSIZE - CF_INBAND_OFFSET bytes (the most a single transaction can carry). Any VAR request admitted by the literals ACL therefore wrote 8 bytes past the end of the handler's stack buffer. Pass the buffer size into GetServerLiteral(), as DoExec2() already does, and bound the writes to it. Both callers pass sizeof(sendbuffer). Changelog: Title Signed-off-by: Dhanraj E --- cf-serverd/server_classic.c | 2 +- cf-serverd/server_common.c | 10 +++---- cf-serverd/server_common.h | 2 +- cf-serverd/server_tls.c | 2 +- tests/unit/protocol_test.c | 54 ++++++++++++++++++++++++++++++++++++- 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/cf-serverd/server_classic.c b/cf-serverd/server_classic.c index 8fc439cb10b..3f05f32552a 100644 --- a/cf-serverd/server_classic.c +++ b/cf-serverd/server_classic.c @@ -1538,7 +1538,7 @@ bool BusyWithClassicConnection(EvalContext *ctx, ServerConnectionState *conn) return false; } - GetServerLiteral(ctx, conn, sendbuffer, recvbuffer, encrypted); + GetServerLiteral(ctx, conn, sendbuffer, sizeof(sendbuffer), recvbuffer, encrypted); return true; case PROTOCOL_COMMAND_CONTEXT_SECURE: diff --git a/cf-serverd/server_common.c b/cf-serverd/server_common.c index 3d26e8ecddf..fb4b0b28b22 100644 --- a/cf-serverd/server_common.c +++ b/cf-serverd/server_common.c @@ -925,7 +925,7 @@ bool CompareLocalHash(const char *filename, const unsigned char digest[EVP_MAX_M } } -void GetServerLiteral(EvalContext *ctx, ServerConnectionState *conn, char *sendbuffer, char *recvbuffer, int encrypted) +void GetServerLiteral(EvalContext *ctx, ServerConnectionState *conn, char *sendbuffer, size_t sendbuffer_size, char *recvbuffer, int encrypted) { char handle[CF_BUFSIZE], out[CF_BUFSIZE]; int cipherlen; @@ -934,13 +934,13 @@ void GetServerLiteral(EvalContext *ctx, ServerConnectionState *conn, char *sendb if (ReturnLiteralData(ctx, handle, out)) { - memset(sendbuffer, 0, CF_BUFSIZE); - snprintf(sendbuffer, CF_BUFSIZE, "%s", out); + memset(sendbuffer, 0, sendbuffer_size); + snprintf(sendbuffer, sendbuffer_size, "%s", out); } else { - memset(sendbuffer, 0, CF_BUFSIZE); - snprintf(sendbuffer, CF_BUFSIZE, "BAD: Not found"); + memset(sendbuffer, 0, sendbuffer_size); + snprintf(sendbuffer, sendbuffer_size, "BAD: Not found"); } if (encrypted) diff --git a/cf-serverd/server_common.h b/cf-serverd/server_common.h index 4e41925e0b7..ab764ef95d8 100644 --- a/cf-serverd/server_common.h +++ b/cf-serverd/server_common.h @@ -48,7 +48,7 @@ int StatFile(ServerConnectionState *conn, char *sendbuffer, char *ofilename); void ReplyServerContext(ServerConnectionState *conn, int encrypted, Item *classes); int CfOpenDirectory(ServerConnectionState *conn, char *sendbuffer, char *oldDirname); int CfSecOpenDirectory(ServerConnectionState *conn, char *sendbuffer, char *dirname); -void GetServerLiteral(EvalContext *ctx, ServerConnectionState *conn, char *sendbuffer, char *recvbuffer, int encrypted); +void GetServerLiteral(EvalContext *ctx, ServerConnectionState *conn, char *sendbuffer, size_t sendbuffer_size, char *recvbuffer, int encrypted); bool GetServerQuery(ServerConnectionState *conn, char *recvbuffer, int encrypted); bool CompareLocalHash(const char *filename, const unsigned char digest[EVP_MAX_MD_SIZE + 1], char sendbuffer[CFD_FALSE_SIZE]); diff --git a/cf-serverd/server_tls.c b/cf-serverd/server_tls.c index c89ad3b4ab3..cae1e3bda02 100644 --- a/cf-serverd/server_tls.c +++ b/cf-serverd/server_tls.c @@ -1003,7 +1003,7 @@ bool BusyWithNewProtocol(EvalContext *ctx, ServerConnectionState *conn) return true; } - GetServerLiteral(ctx, conn, sendbuffer, recvbuffer, encrypted); + GetServerLiteral(ctx, conn, sendbuffer, sizeof(sendbuffer), recvbuffer, encrypted); return true; } case PROTOCOL_COMMAND_CONTEXT: diff --git a/tests/unit/protocol_test.c b/tests/unit/protocol_test.c index 29b30d31a5d..39829e3a694 100644 --- a/tests/unit/protocol_test.c +++ b/tests/unit/protocol_test.c @@ -8,6 +8,13 @@ #include /* GetCommandClassic */ +#include +#include +#include + +#include +#include + /* * The protocol consists of the following commands: @@ -450,13 +457,58 @@ static void test_user_name(void) assert_true(IsUserNameValid(valid_user_name)); } +/* + * BusyWithNewProtocol() hands GetServerLiteral() a sendbuffer of only + * CF_BUFSIZE - CF_INBAND_OFFSET bytes. The function must not write past the + * size it is given, so guard bytes placed right after a buffer of that size + * have to survive the call. + */ +static void test_get_server_literal_sendbuffer_bound(void) +{ + int sv[2]; + assert_int_equal(socketpair(AF_UNIX, SOCK_STREAM, 0, sv), 0); + + ServerConnectionState *conn = xcalloc(1, sizeof(*conn)); + conn->conn_info = ConnectionInfoNew(); + ConnectionInfoSetSocket(conn->conn_info, sv[0]); + ConnectionInfoSetProtocolVersion(conn->conn_info, CF_PROTOCOL_CLASSIC); + + ConnectionInfo *client = ConnectionInfoNew(); + ConnectionInfoSetSocket(client, sv[1]); + ConnectionInfoSetProtocolVersion(client, CF_PROTOCOL_CLASSIC); + + EvalContext *ctx = EvalContextNew(); + + char sendbuffer[CF_MSGSIZE + CF_INBAND_OFFSET]; + memset(sendbuffer, 0x5a, sizeof(sendbuffer)); + + char recvbuffer[] = "VAR literal_handle"; + GetServerLiteral(ctx, conn, sendbuffer, CF_MSGSIZE, recvbuffer, 0); + + for (size_t i = CF_MSGSIZE; i < sizeof(sendbuffer); i++) + { + assert_int_equal(sendbuffer[i], 0x5a); + } + + char reply[CF_BUFSIZE]; + int len = ReceiveTransaction(client, reply, NULL); + assert_true(len > 0); + assert_string_equal(reply, "BAD: Not found"); + + EvalContextDestroy(ctx); + ConnectionInfoDestroy(&client); + ConnectionInfoDestroy(&conn->conn_info); + free(conn); +} + int main() { PRINT_TEST_BANNER(); const UnitTest tests[] = { unit_test(test_command_parser), - unit_test(test_user_name) + unit_test(test_user_name), + unit_test(test_get_server_literal_sendbuffer_bound) }; return run_tests(tests);