upstream

[kooksee]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces major enhancements to the xfrpc client, including the integration of wolfSSL and ngtcp2 for QUIC transport support, NAT hole punching (XTCP) for P2P traversal, TCP/HTTP health checks, Unix Domain Socket (UDS) proxying, and SIGHUP-triggered configuration hot-reloading. The review identified several critical and high-severity issues across the new transport and proxy implementations: potential data truncation and protocol corruption due to unhandled partial writes or EAGAIN when writing directly to non-blocking sockets; a lack of upper-bound validation on incoming message lengths which could cause out-of-memory (OOM) conditions; unhandled partial writes in the QUIC client transport leading to high latency; leaked QUIC streams on socketpair closure; and a permanent leak of work stream slots. Additionally, potential buffer overflows from truncated Unix socket paths and unbounded memory growth in HTTP health checks were flagged.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Writing directly to a non-blocking socket using write without handling partial writes or EAGAIN can lead to silent data truncation and protocol corruption. If the socket buffer is full, write will return EAGAIN or write fewer bytes than requested, and the unwritten bytes will be silently discarded. Consider using a loop to handle partial writes or utilizing evbuffer / bufferevent APIs to manage the buffering safely.

[gemini-code-assist]

Casting msg->length (which is received from the remote server) to a signed int without any upper-bound validation can lead to integer overflow or out-of-memory (OOM) denial of service. If a malformed or malicious packet specifies a very large length, malloc might fail or, worse, lead to buffer overflows if signed/unsigned mismatches occur in subsequent operations. A sanity check should be added to ensure the length is within a reasonable limit (e.g., < 10MB).

	uint32_t data_len = msg_hton(msg->length);
	if (data_len > 10 * 1024 * 1024) {
		debug(LOG_ERR, "Received message length too large: %u", data_len);
		return;
	}
	char *json_str = malloc(data_len + 1);
	if (!json_str) return;
	memcpy(json_str, msg->data, data_len);
	json_str[data_len] = '\0';

[gemini-code-assist]

evbuffer_write can perform a partial write if the socket's write buffer is full. When this happens, the remaining data stays in qc->quic_out_buf. However, there is no write event registered on qc->sp_fd to resume flushing when the socket becomes writeable again. The remaining data will remain stuck until the next UDP read event triggers another flush, causing high latency or hangs.

[gemini-code-assist]

If read returns 0 (EOF) or a negative value (excluding EAGAIN/EWOULDBLOCK), it indicates that the local side of the socketpair has closed. Currently, the callback simply returns without notifying the remote QUIC peer, leading to a leaked QUIC stream. The stream should be explicitly shut down using ngtcp2_conn_shutdown_stream.

[gemini-code-assist]

When a work stream is closed in qc_stream_close, its resources are freed, but the slot in qc->work_streams is not removed, and qc->work_stream_count is never decremented. This creates a permanent leak of stream slots, meaning a single QUIC connection can only ever handle 64 work streams before failing with "QUIC: too many work streams". Reusing empty slots (where sp_fd == -1) resolves this limitation.

	struct work_stream_relay *relay = NULL;
	for (int i = 0; i < qc->work_stream_count; i++) {
		if (qc->work_streams[i].sp_fd < 0) {
			relay = &qc->work_streams[i];
			break;
		}
	}
	if (!relay) {
		if (qc->work_stream_count >= 64) {
			debug(LOG_ERR, "QUIC: too many work streams");
			close(sp[0]); close(sp[1]);
			return NULL;
		}
		relay = &qc->work_streams[qc->work_stream_count++];
	}
	relay->stream_id = sid;
	relay->sp_fd = sp[0];
	relay->out_buf = evbuffer_new();

[gemini-code-assist]

If unix_path exceeds sizeof(sun.sun_path) - 1 (typically 107 bytes on Linux), strncpy will truncate the path. Connecting to a truncated path will fail or connect to the wrong socket. A length check should be added to reject paths that are too long.

	if (strlen(unix_path) >= sizeof(sun.sun_path)) {
		debug(LOG_ERR, "Unix socket path too long: %s", unix_path);
		close(fd);
		return NULL;
	}
	struct sockaddr_un sun;
	memset(&sun, 0, sizeof(sun));
	sun.sun_family = AF_UNIX;
	strncpy(sun.sun_path, unix_path, sizeof(sun.sun_path) - 1);

[gemini-code-assist]

If the HTTP server sends a very long status line or keeps sending data without a \r\n, the input buffer will grow indefinitely, leading to high memory usage or OOM. A maximum limit (e.g., 4096 bytes) should be enforced on the input buffer length before failing the health check.

	struct evbuffer *input = bufferevent_get_input(bev);
	if (evbuffer_get_length(input) > 4096) {
		debug(LOG_WARNING, "Health check [%s]: HTTP status line too long", entry->proxy_name);
		hc_finish_check(entry, 0);
		return;
	}
	struct evbuffer_ptr crlf = evbuffer_search(input, "\r\n", 2, NULL);