diff --git a/quiche/src/frame.rs b/quiche/src/frame.rs index 3ecfcd3707..ba5d9cb2e1 100644 --- a/quiche/src/frame.rs +++ b/quiche/src/frame.rs @@ -261,15 +261,25 @@ impl Frame { limit: b.get_varint()?, }, - 0x18 => Frame::NewConnectionId { - seq_num: b.get_varint()?, - retire_prior_to: b.get_varint()?, - conn_id: b.get_bytes_with_u8_length()?.to_vec(), - reset_token: b - .get_bytes(16)? - .buf() - .try_into() - .map_err(|_| Error::BufferTooShort)?, + 0x18 => { + let seq_num = b.get_varint()?; + let retire_prior_to = b.get_varint()?; + let conn_id_len = b.get_u8()?; + + if !(1..=packet::MAX_CID_LEN).contains(&conn_id_len) { + return Err(Error::InvalidFrame); + } + + Frame::NewConnectionId { + seq_num, + retire_prior_to, + conn_id: b.get_bytes(conn_id_len as usize)?.to_vec(), + reset_token: b + .get_bytes(16)? + .buf() + .try_into() + .map_err(|_| Error::BufferTooShort)?, + } }, 0x19 => Frame::RetireConnectionId { diff --git a/quiche/src/lib.rs b/quiche/src/lib.rs index c0286e4102..9ad8da6e6d 100644 --- a/quiche/src/lib.rs +++ b/quiche/src/lib.rs @@ -14630,6 +14630,147 @@ mod tests { ); } + #[test] + /// Tests that NEW_CONNECTION_ID with zero-length CID are rejected. + fn connection_id_zero() { + let mut buf = [0; 65535]; + + let mut config = Config::new(crate::PROTOCOL_VERSION).unwrap(); + config + .load_cert_chain_from_pem_file("examples/cert.crt") + .unwrap(); + config + .load_priv_key_from_pem_file("examples/cert.key") + .unwrap(); + config + .set_application_protos(&[b"proto1", b"proto2"]) + .unwrap(); + config.verify_peer(false); + config.set_active_connection_id_limit(2); + + let mut pipe = testing::Pipe::with_config(&mut config).unwrap(); + assert_eq!(pipe.handshake(), Ok(())); + + let mut frames = Vec::new(); + + // Client adds a CID that is too short. + let (scid, reset_token) = testing::create_cid_and_reset_token(0); + + frames.push(frame::Frame::NewConnectionId { + seq_num: 1, + retire_prior_to: 0, + conn_id: scid.to_vec(), + reset_token: reset_token.to_be_bytes(), + }); + + let pkt_type = packet::Type::Short; + + let written = + testing::encode_pkt(&mut pipe.client, pkt_type, &frames, &mut buf) + .unwrap(); + + let active_path = pipe.server.paths.get_active().unwrap(); + let info = RecvInfo { + to: active_path.local_addr(), + from: active_path.peer_addr(), + }; + + assert_eq!( + pipe.server.recv(&mut buf[..written], info), + Err(Error::InvalidFrame) + ); + + let written = match pipe.server.send(&mut buf) { + Ok((write, _)) => write, + + Err(_) => unreachable!(), + }; + + let frames = + testing::decode_pkt(&mut pipe.client, &mut buf[..written]).unwrap(); + let mut iter = frames.iter(); + + assert_eq!( + iter.next(), + Some(&frame::Frame::ConnectionClose { + error_code: 0x7, + frame_type: 0, + reason: Vec::new(), + }) + ); + } + + #[test] + /// Tests that NEW_CONNECTION_ID with too long CID are rejected. + fn connection_id_invalid_max_len() { + let mut buf = [0; 65535]; + + let mut config = Config::new(crate::PROTOCOL_VERSION).unwrap(); + config + .load_cert_chain_from_pem_file("examples/cert.crt") + .unwrap(); + config + .load_priv_key_from_pem_file("examples/cert.key") + .unwrap(); + config + .set_application_protos(&[b"proto1", b"proto2"]) + .unwrap(); + config.verify_peer(false); + config.set_active_connection_id_limit(2); + + let mut pipe = testing::Pipe::with_config(&mut config).unwrap(); + assert_eq!(pipe.handshake(), Ok(())); + + let mut frames = Vec::new(); + + // Client adds a CID that is too long. + let (scid, reset_token) = + testing::create_cid_and_reset_token(MAX_CONN_ID_LEN + 1); + + frames.push(frame::Frame::NewConnectionId { + seq_num: 1, + retire_prior_to: 0, + conn_id: scid.to_vec(), + reset_token: reset_token.to_be_bytes(), + }); + + let pkt_type = packet::Type::Short; + + let written = + testing::encode_pkt(&mut pipe.client, pkt_type, &frames, &mut buf) + .unwrap(); + + let active_path = pipe.server.paths.get_active().unwrap(); + let info = RecvInfo { + to: active_path.local_addr(), + from: active_path.peer_addr(), + }; + + assert_eq!( + pipe.server.recv(&mut buf[..written], info), + Err(Error::InvalidFrame) + ); + + let written = match pipe.server.send(&mut buf) { + Ok((write, _)) => write, + + Err(_) => unreachable!(), + }; + + let frames = + testing::decode_pkt(&mut pipe.client, &mut buf[..written]).unwrap(); + let mut iter = frames.iter(); + + assert_eq!( + iter.next(), + Some(&frame::Frame::ConnectionClose { + error_code: 0x7, + frame_type: 0, + reason: Vec::new(), + }) + ); + } + #[test] /// Exercises the handling of NEW_CONNECTION_ID and RETIRE_CONNECTION_ID /// frames.