From: Stefan Sperling <stsp@stsp.name>
Subject: gotd flush packets
To: gameoftrees@openbsd.org
Date: Fri, 20 Jan 2023 19:57:59 +0100

Someone did a silly thing against our public git mirror:

Jan 20 19:20:23 florabel gotd[74429]: received flush-pkt from uid 1002
Jan 20 19:20:43 florabel last message repeated 251236 times
Jan 20 19:20:43 florabel gotd[70339]: list-refs request from uid 1002
Jan 20 19:20:43 florabel gotd[74429]: received flush-pkt from uid 1002
Jan 20 19:21:14 florabel last message repeated 386788 times
Jan 20 19:21:43 florabel last message repeated 367291 times

There can of course be various variations of something like this in the
git protocol, so the patch below is not fool proof. But it is relatively
easy to catch the above case and we have to start somehwere.
If you are the person who triggered this, please let me know if you
are willing to help with developing countermeasures for such things,
It would be fun and be more productive use of your time. We should have
tests for such cases in our test suite to start with.

As far as I understand the Git protocol docs there are only three places
where we expect a flush packet from the client:

 - After sending our ref announcement we can receive a flush packet
   if the client decides it does not want anything.
 - The list of want lines during fetch is terminated by a flush packet.
 - The list of ref-updates lines during send is terminated by a flush packet.

The test suite is happy and trivial manual fetch/send testing suggests
both fetch and send still works with both got and git clients.

ok?

 make gotd accept just one flush packet at a time
 
diff 7713cc5e4f5544e81909670d592e89526ed86c9b 9859bfe6141544a12425fb6a7d391b1c6502ea15
commit - 7713cc5e4f5544e81909670d592e89526ed86c9b
commit + 9859bfe6141544a12425fb6a7d391b1c6502ea15
blob - 0bc48fac834b679822e3314db38590b9da8f3575
blob + 48373a42189877c22d054eec8a3f0d47791909e4
--- gotd/session.c
+++ gotd/session.c
@@ -79,6 +79,7 @@ static struct gotd_session_client {
 	char				*packfile_path;
 	char				*packidx_path;
 	int				 nref_updates;
+	int				 accept_flush_pkt;
 } gotd_session_client;
 
 void gotd_session_sighdlr(int sig, short event, void *arg);
@@ -1037,10 +1038,12 @@ session_dispatch_listener(int fd, short events, void *
 				break;
 			if (!client->is_writing) {
 				client->state = GOTD_STATE_EXPECT_WANT;
+				client->accept_flush_pkt = 1;
 				log_debug("uid %d: expecting want-lines",
 				    client->euid);
 			} else if (client->is_writing) {
 				client->state = GOTD_STATE_EXPECT_REF_UPDATE;
+				client->accept_flush_pkt = 1;
 				log_debug("uid %d: expecting ref-update-lines",
 				    client->euid);
 			} else
@@ -1109,6 +1112,19 @@ session_dispatch_listener(int fd, short events, void *
 				    "unexpected flush-pkt received");
 				break;
 			}
+			if (!client->accept_flush_pkt) {
+				err = got_error_msg(GOT_ERR_BAD_REQUEST,
+				    "unexpected flush-pkt received");
+				break;
+			}
+
+			/*
+			 * Accept just one flush packet at a time.
+			 * Future client state transitions will set this flag
+			 * again if another flush packet is expected.
+			 */
+			client->accept_flush_pkt = 0;
+
 			log_debug("received flush-pkt from uid %d",
 			    client->euid);
 			if (client->state == GOTD_STATE_EXPECT_WANT) {
@@ -1402,6 +1418,7 @@ session_main(const char *title, const char *repo_path,
 	gotd_session_client.fd = -1;
 	gotd_session_client.nref_updates = -1;
 	gotd_session_client.delta_cache_fd = -1;
+	gotd_session_client.accept_flush_pkt = 1;
 
 	imsg_init(&gotd_session.parent_iev.ibuf, GOTD_FILENO_MSG_PIPE);
 	gotd_session.parent_iev.handler = session_dispatch;