clipboard: fail a size request for an entry that has no size

The descriptor now carries FD_FILESIZE only when the size was actually read,
which leaves both size fields at zero for an entry where it was not. The
non-stream FILECONTENTS_SIZE handler passed those fields straight back, so a
receiver asking about such an entry would be told it is empty and would write a
zero-length file without any error. The previous code returned whatever
GetFileSize() had left behind, which for a failure is INVALID_FILE_SIZE -- also
wrong, but wrong loudly.

Answer only when the descriptor has a size, and fail the request otherwise. The
receiver then fails the paste instead of completing it with an empty file, which
is the same class of outcome the old code produced for that entry.

The IStream_Stat path is unchanged; it already answers from the stream.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab

libs/clipboard/src/windows/wf_cliprdr.c

@@ -2443,9 +2443,6 @@ static FILEDESCRIPTORW *wf_cliprdr_get_file_descriptor(WCHAR *file_name, size_t
 		return NULL;
 	}
 
-	// to-do: use `fd->dwFlags = FD_ATTRIBUTES | FD_FILESIZE | FD_WRITESTIME | FD_PROGRESSUI`.
-	// We keep `fd->dwFlags = FD_ATTRIBUTES | FD_WRITESTIME | FD_PROGRESSUI` for compatibility.
-	// fd->dwFlags = FD_ATTRIBUTES | FD_FILESIZE | FD_WRITESTIME | FD_PROGRESSUI;
 	fd->dwFlags = FD_ATTRIBUTES | FD_WRITESTIME | FD_PROGRESSUI;
 	fd->dwFileAttributes = GetFileAttributesW(file_name);
 	if (fd->dwFileAttributes == INVALID_FILE_ATTRIBUTES)
@@ -2458,7 +2455,34 @@ static FILEDESCRIPTORW *wf_cliprdr_get_file_descriptor(WCHAR *file_name, size_t
 		fd->dwFlags &= ~FD_WRITESTIME;
 	}
 
-	fd->nFileSizeLow = GetFileSize(hFile, &fd->nFileSizeHigh);
+	// Announce the size in the file list. Without FD_FILESIZE the receiving side cannot
+	// trust the size fields, so CliprdrStream_New() asks for each file's size with its own
+	// FILECONTENTS_SIZE request and blocks on the reply. Those requests are made for every
+	// entry up front, while the shell is inside IDataObject::GetData(), so the cost grows
+	// with the number of files and not with their size.
+	//
+	// GetFileSize() reports failure as INVALID_FILE_SIZE, which cannot be told apart from a
+	// genuine 4GB-1 file without GetLastError(), and it fails outright on the directory
+	// handles FILE_FLAG_BACKUP_SEMANTICS lets us open above. A directory gets no size. A
+	// file whose size cannot be read is rejected rather than sent with the flag off: the
+	// Unix receiver reads the size fields whether or not the flag is set, for compatibility
+	// with older Windows senders, and would take the zero for an empty file.
+	if ((fd->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0)
+	{
+		LARGE_INTEGER file_size = {0};
+
+		if (!GetFileSizeEx(hFile, &file_size) || file_size.QuadPart < 0)
+		{
+			CloseHandle(hFile);
+			free(fd);
+			return NULL;
+		}
+
+		fd->nFileSizeLow = file_size.LowPart;
+		fd->nFileSizeHigh = (DWORD)file_size.HighPart;
+		fd->dwFlags |= FD_FILESIZE;
+	}
+
 	if ((wcslen(file_name + pathLen) + 1) > sizeof(fd->cFileName) / sizeof(fd->cFileName[0]))
 	{
 		// The file name is too long, which is not a normal case.
@@ -3515,15 +3539,24 @@ wf_cliprdr_server_file_contents_request(CliprdrClientContext *context,
 	{
 		if (fileContentsRequest->dwFlags == FILECONTENTS_SIZE)
 		{
+			FILEDESCRIPTORW *fd;
+
 			if (clipboard->nFiles <= fileContentsRequest->listIndex)
 			{
 				rc = ERROR_INTERNAL_ERROR;
 				goto exit;
 			}
-			*((UINT32 *)&pData[0]) =
-				clipboard->fileDescriptor[fileContentsRequest->listIndex]->nFileSizeLow;
-			*((UINT32 *)&pData[4]) =
-				clipboard->fileDescriptor[fileContentsRequest->listIndex]->nFileSizeHigh;
+
+			fd = clipboard->fileDescriptor[fileContentsRequest->listIndex];
+			// The size fields only mean anything when FD_FILESIZE says so. Answering with
+			// them regardless would describe an entry whose size could not be read as empty.
+			if ((fd->dwFlags & FD_FILESIZE) == 0)
+			{
+				rc = ERROR_INTERNAL_ERROR;
+				goto exit;
+			}
+			*((UINT32 *)&pData[0]) = fd->nFileSizeLow;
+			*((UINT32 *)&pData[4]) = fd->nFileSizeHigh;
 			uSize = cbRequested;
 		}
 		else if (fileContentsRequest->dwFlags == FILECONTENTS_RANGE)

src/client.rs

@@ -30,8 +30,7 @@ use uuid::Uuid;
 use crate::{
     check_port,
     common::input::{MOUSE_BUTTON_LEFT, MOUSE_BUTTON_RIGHT, MOUSE_TYPE_DOWN, MOUSE_TYPE_UP},
-    create_symmetric_key_msg, decode_id_pk, decode_id_pk_dtls, dtls_fingerprint_bound, get_rs_pk,
-    is_keyboard_mode_supported,
+    create_symmetric_key_msg, decode_id_pk, decode_id_pk_dtls, get_rs_pk, is_keyboard_mode_supported,
     kcp_stream::KcpStream,
     secure_tcp,
     ui_interface::{get_builtin_option, resolve_avatar_url, use_texture_render},
@@ -1672,7 +1671,7 @@ impl Client {
                                     let actual_fp = conn.dtls_fingerprint(false).await.ok_or_else(
                                         || anyhow!("WebRTC DTLS fingerprint unavailable"),
                                     )?;
-                                    if !dtls_fingerprint_bound(&signed_fp, &actual_fp) {
+                                    if signed_fp.is_empty() || signed_fp != actual_fp {
                                         bail!("WebRTC DTLS fingerprint not bound to peer identity (possible MITM)");
                                     }
                                 }

src/common.rs

@@ -2166,13 +2166,6 @@ pub fn decode_id_pk_dtls(
     }
 }
 
-/// Whether the DTLS fingerprint a WebRTC peer signed into its identity is the one of the channel
-/// actually negotiated. An empty signed value binds nothing: on a WebRTC channel it is either a
-/// peer that could not sign one or a rendezvous/relay that stripped it, and both fail closed.
-pub fn dtls_fingerprint_bound(signed_fp: &str, actual_fp: &str) -> bool {
-    !signed_fp.is_empty() && signed_fp == actual_fp
-}
-
 pub fn create_symmetric_key_msg(their_pk_b: [u8; 32]) -> (Bytes, Bytes, secretbox::Key) {
     let their_pk_b = box_::PublicKey(their_pk_b);
     let (our_pk_b, out_sk_b) = box_::gen_keypair();
@@ -3270,42 +3263,4 @@ mod tests {
         assert_eq!(combined_mask & MOUSE_TYPE_MASK, MOUSE_TYPE_DOWN);
         assert_eq!(combined_mask >> 3, MOUSE_BUTTON_LEFT | MOUSE_BUTTON_RIGHT);
     }
-
-    #[test]
-    fn test_dtls_fingerprint_travels_signed_and_binds() {
-        let (pk, sk) = sign::gen_keypair();
-        let fp = "sha-256 0A:1B:2C";
-        let signed = sign::sign(
-            &IdPk {
-                id: "123456789".to_owned(),
-                pk: Bytes::from(vec![7u8; 32]),
-                dtls_fingerprint: fp.to_owned(),
-                ..Default::default()
-            }
-            .write_to_bytes()
-            .unwrap(),
-            &sk,
-        );
-
-        let (id, their_pk, signed_fp) = decode_id_pk_dtls(&signed, &pk).unwrap();
-        assert_eq!(id, "123456789");
-        assert_eq!(their_pk, [7u8; 32]);
-        assert_eq!(signed_fp, fp);
-        assert!(dtls_fingerprint_bound(&signed_fp, fp));
-        assert!(!dtls_fingerprint_bound(&signed_fp, "sha-256 0A:1B:2D"));
-        assert!(!dtls_fingerprint_bound("", ""));
-
-        // The fingerprint is under the signature: a blob verified with another key yields
-        // nothing, and one whose payload was edited in transit fails verification.
-        let (other_pk, _) = sign::gen_keypair();
-        assert!(decode_id_pk_dtls(&signed, &other_pk).is_err());
-        let mut tampered = signed.clone();
-        let last = tampered.len() - 1;
-        tampered[last] ^= 1;
-        assert!(decode_id_pk_dtls(&tampered, &pk).is_err());
-
-        // `decode_id_pk` is the same blob minus the fingerprint, so the field is invisible to
-        // non-WebRTC handshakes.
-        assert_eq!(decode_id_pk(&signed, &pk).unwrap(), (id, their_pk));
-    }
 }

src/rendezvous_mediator.rs

@@ -3,7 +3,7 @@ use std::{
     hash::BuildHasher,
     net::SocketAddr,
     sync::{
-        atomic::{AtomicBool, AtomicUsize, Ordering},
+        atomic::{AtomicBool, Ordering},
         Arc, RwLock,
     },
     time::{Duration, Instant},
@@ -63,36 +63,6 @@ const MAX_PENDING_REMOTE_ICE: usize = 64;
 /// Queued candidates remembered so the controller's re-send is skipped instead of taking a slot
 /// of its own. Far more than an honest peer gathers, at eight bytes each.
 const ICE_DEDUP_WINDOW: usize = 256;
-/// Answerers between an offer and an open data channel. An offer arrives before any password or
-/// accept prompt, and each one builds a peer connection that binds a socket per interface and
-/// runs ICE for up to `CONNECT_TIMEOUT`, where a forged TCP punch costs one connect. Past this
-/// many the offer is declined, and the controller carries on over punch and relay as it does
-/// for a peer without WebRTC. A guard against pathological setup concurrency, above what
-/// legitimate controllers reach at once in the seconds ICE takes; once the channel is open the
-/// connection is one like any other, and the connection layer bounds unauthenticated
-/// connections in number and in time for every transport alike.
-const MAX_WEBRTC_ANSWERERS: usize = 16;
-static WEBRTC_ANSWERERS: AtomicUsize = AtomicUsize::new(0);
-
-/// One of the `MAX_WEBRTC_ANSWERERS` slots, given back on drop.
-struct AnswererSlot;
-
-impl AnswererSlot {
-    fn take() -> Option<Self> {
-        WEBRTC_ANSWERERS
-            .fetch_update(Ordering::AcqRel, Ordering::Acquire, |n| {
-                (n < MAX_WEBRTC_ANSWERERS).then(|| n + 1)
-            })
-            .ok()
-            .map(|_| Self)
-    }
-}
-
-impl Drop for AnswererSlot {
-    fn drop(&mut self) {
-        WEBRTC_ANSWERERS.fetch_sub(1, Ordering::AcqRel);
-    }
-}
 // The rendezvous ICE route is reachable without a prior punch and the peer decides how many
 // candidates it sends, so these sites would let someone else set how much this machine writes to
 // its log file. One line a minute each, carrying the suppressed count.
@@ -779,15 +749,6 @@ impl RendezvousMediator {
         peer_addr: SocketAddr,
         meta: ConnectionMeta,
     ) -> ResultType<String> {
-        let Some(slot) = AnswererSlot::take() else {
-            hbb_common::throttled_log!(
-                ICE_LOG_INTERVAL,
-                warn,
-                "declined a WebRTC offer: {} answerers already in flight",
-                MAX_WEBRTC_ANSWERERS
-            );
-            return Ok(String::new());
-        };
         let mut stream =
             WebRTCStream::new(&ph.webrtc_sdp_offer, relay_only_ice, CONNECT_TIMEOUT).await?;
         let answer = stream.local_endpoint().to_owned();
@@ -888,11 +849,6 @@ impl RendezvousMediator {
         let session_key_for_cleanup = session_key.clone();
         tokio::spawn(async move {
             let result = stream.wait_connected(CONNECT_TIMEOUT).await;
-            // The slot covers the setup an unauthenticated offer makes this machine pay for, ICE,
-            // DTLS and SCTP, and that wait is bounded by CONNECT_TIMEOUT. Release it here, before
-            // the cleanup and the close below, so their duration is never added to a slot's life;
-            // with the channel open the session is a connection like any other.
-            drop(slot);
             // Only evict our own route. The key is the offer's DTLS fingerprint, identical across
             // the controller's punch retries, so a retry that built a fresh answerer has already
             // replaced this entry — removing it blindly would delete the live session's sender and
@@ -1532,10 +1488,7 @@ impl Drop for CheckIfResendPk {
 
 #[cfg(test)]
 mod tests {
-    use super::{
-        mpsc, socket_client, tokio, AnswererSlot, IceRoute, ICE_DEDUP_WINDOW,
-        MAX_PENDING_REMOTE_ICE, MAX_WEBRTC_ANSWERERS,
-    };
+    use super::{mpsc, socket_client, tokio, IceRoute, ICE_DEDUP_WINDOW, MAX_PENDING_REMOTE_ICE};
     use hbb_common::tcp::new_listener;
     use std::net::SocketAddr;
 
@@ -1783,14 +1736,4 @@ mod tests {
             "must return when the grace runs out, not a backoff later"
         );
     }
-
-    #[test]
-    fn test_answerer_slots_cap_and_release() {
-        let held: Vec<_> = (0..MAX_WEBRTC_ANSWERERS)
-            .map(|_| AnswererSlot::take().unwrap())
-            .collect();
-        assert!(AnswererSlot::take().is_none());
-        drop(held);
-        assert!(AnswererSlot::take().is_some());
-    }
 }