Fixed stack buffer overflow in GetServerLiteral for TLS VAR requests

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:

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)

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]);

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:

tests/unit/protocol_test.c

@@ -8,6 +8,13 @@
 
 #include <server_classic.c>                            /* GetCommandClassic */
 
+#include <connection_info.h>
+#include <net.h>
+#include <eval_context.h>
+
+#include <sys/socket.h>
+#include <unistd.h>
+
 
 /*
  * 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);