From 2aff5d38e0a7094060e4e4f067dbb5fffad1b653 Mon Sep 17 00:00:00 2001 From: Edwin Wu Date: Sat, 11 Jul 2026 06:04:31 -0700 Subject: [PATCH] Address review comments on port validation. PiperOrigin-RevId: 946184592 --- .../mediums/wifi_hotspot_bwu_handler.cc | 7 +- .../mediums/wifi_hotspot_bwu_handler_test.cc | 49 +++- .../mediums/wifi_lan_bwu_handler.cc | 9 +- .../mediums/wifi_lan_bwu_handler_test.cc | 27 +++ .../offline_frames_validator.cc | 59 ++++- .../offline_frames_validator_test.cc | 221 +++++++++++++++++- internal/platform/service_address.cc | 4 +- internal/platform/service_address_test.cc | 12 +- 8 files changed, 363 insertions(+), 25 deletions(-) diff --git a/connections/implementation/mediums/wifi_hotspot_bwu_handler.cc b/connections/implementation/mediums/wifi_hotspot_bwu_handler.cc index d31b8a15..7f9267f9 100644 --- a/connections/implementation/mediums/wifi_hotspot_bwu_handler.cc +++ b/connections/implementation/mediums/wifi_hotspot_bwu_handler.cc @@ -194,7 +194,9 @@ WifiHotspotBwuHandler::CreateUpgradedEndpointChannel( } // Add gateway and port to address candidates if address candidates is empty. if (service_addresses.empty() && - upgrade_path_info_credentials.has_gateway()) { + upgrade_path_info_credentials.has_gateway() && + upgrade_path_info_credentials.port() > 0 && + upgrade_path_info_credentials.port() <= 65535) { std::vector address_bytes = GatewayToAddressBytes(upgrade_path_info_credentials.gateway()); if (!address_bytes.empty()) { @@ -230,7 +232,8 @@ WifiHotspotBwuHandler::CreateUpgradedEndpointChannel( LOG(ERROR) << "WifiHotspotBwuHandler failed to connect to the WifiHotspot " "service for endpoint " << endpoint_id; - return {Error(socket_result.error().operation_result_code().value())}; + return {Error(socket_result.error().operation_result_code().value_or( + OperationResultCode::DETAIL_UNKNOWN))}; } VLOG(1) << "WifiHotspotBwuHandler successfully connected to WifiHotspot service " diff --git a/connections/implementation/mediums/wifi_hotspot_bwu_handler_test.cc b/connections/implementation/mediums/wifi_hotspot_bwu_handler_test.cc index fcd42428..b30513af 100644 --- a/connections/implementation/mediums/wifi_hotspot_bwu_handler_test.cc +++ b/connections/implementation/mediums/wifi_hotspot_bwu_handler_test.cc @@ -55,8 +55,9 @@ class WifiHotspotTest : public testing::Test { ~WifiHotspotTest() override { env_.Stop(); } void SetUp() override { nearby::NearbyFlags::GetInstance().OverrideInt64FlagValue( - platform::config_package_nearby::nearby_platform_feature:: - kWifiHotspotConnectionIntervalMillis, 1); + platform::config_package_nearby::nearby_platform_feature:: + kWifiHotspotConnectionIntervalMillis, + 1); } void TearDown() override { nearby::NearbyFlags::GetInstance().ResetOverridedValues(); @@ -89,9 +90,10 @@ TEST_F(WifiHotspotTest, SoftAPBWUInit_STACreateEndpointChannel) { ExceptionOr upgrade_frame; auto handler_1 = std::make_unique( - &mediums_HS_ap.GetWifiHotspot(), [&](ClientProxy* client, - std::unique_ptr - mutable_connection) { + &mediums_HS_ap.GetWifiHotspot(), + [&](ClientProxy* client, + std::unique_ptr + mutable_connection) { LOG(INFO) << "Server socket connection accept call back, Socket name: " << mutable_connection->socket->ToString(); accept_latch.CountDown(); @@ -169,5 +171,42 @@ TEST_F(WifiHotspotTest, SoftAPBWUInit_STACreateEndpointChannel) { EXPECT_FALSE(mediums_HS_sta.GetWifiHotspot().IsConnectedToHotspot()); } +TEST_F(WifiHotspotTest, CreateUpgradedEndpointChannel_RejectGatewayPort0) { + ClientProxy client; + client.AddCancellationFlag(std::string(kEndpointID)); + Mediums mediums; + WifiHotspotBwuHandler handler(&mediums.GetWifiHotspot(), nullptr); + + UpgradePathInfo path_info; + auto* credentials = path_info.mutable_wifi_hotspot_credentials(); + credentials->set_ssid("SSID"); + credentials->set_password("password"); + credentials->set_gateway("192.168.43.1"); + + // Port 0 + credentials->set_port(0); + auto result = handler.CreateUpgradedEndpointChannel( + &client, std::string(kServiceID), std::string(kEndpointID), path_info); + EXPECT_TRUE(result.has_error()); + EXPECT_EQ(result.error().operation_result_code().value(), + OperationResultCode::CONNECTIVITY_WIFI_HOTSPOT_INVALID_CREDENTIAL); + + // Port > 65535 + credentials->set_port(65536); + result = handler.CreateUpgradedEndpointChannel( + &client, std::string(kServiceID), std::string(kEndpointID), path_info); + EXPECT_TRUE(result.has_error()); + EXPECT_EQ(result.error().operation_result_code().value(), + OperationResultCode::CONNECTIVITY_WIFI_HOTSPOT_INVALID_CREDENTIAL); + + // Port < 0 + credentials->set_port(-1); + result = handler.CreateUpgradedEndpointChannel( + &client, std::string(kServiceID), std::string(kEndpointID), path_info); + EXPECT_TRUE(result.has_error()); + EXPECT_EQ(result.error().operation_result_code().value(), + OperationResultCode::CONNECTIVITY_WIFI_HOTSPOT_INVALID_CREDENTIAL); +} + } // namespace connections } // namespace nearby diff --git a/connections/implementation/mediums/wifi_lan_bwu_handler.cc b/connections/implementation/mediums/wifi_lan_bwu_handler.cc index 607c5b9f..d5929c97 100644 --- a/connections/implementation/mediums/wifi_lan_bwu_handler.cc +++ b/connections/implementation/mediums/wifi_lan_bwu_handler.cc @@ -89,9 +89,11 @@ WifiLanBwuHandler::CreateUpgradedEndpointChannel( } // Only use ip_address and wifi_port if address_candidates is empty. if (address_candidates.empty()) { - if (upgrade_path_info_socket.ip_address().size() != 4) { + if (upgrade_path_info_socket.ip_address().size() != 4 || + upgrade_path_info_socket.wifi_port() <= 0 || + upgrade_path_info_socket.wifi_port() > 65535) { LOG(ERROR) << "WifiLanBwuHandler: fallback ip_address size is not 4 " - << "(IPv4 only)."; + << "or port is invalid (IPv4 only)."; return { Error(OperationResultCode::CONNECTIVITY_WIFI_LAN_IP_ADDRESS_ERROR)}; } @@ -121,7 +123,8 @@ WifiLanBwuHandler::CreateUpgradedEndpointChannel( LOG(ERROR) << "WifiLanBwuHandler failed to connect to the WifiLan service (" << address_candidate << ") for endpoint " << endpoint_id; - error = Error(socket_result.error().operation_result_code().value()); + error = Error(socket_result.error().operation_result_code().value_or( + OperationResultCode::DETAIL_UNKNOWN)); continue; } VLOG(1) << "WifiLanBwuHandler successfully connected to WifiLan service (" diff --git a/connections/implementation/mediums/wifi_lan_bwu_handler_test.cc b/connections/implementation/mediums/wifi_lan_bwu_handler_test.cc index cf09e7bb..e561640e 100644 --- a/connections/implementation/mediums/wifi_lan_bwu_handler_test.cc +++ b/connections/implementation/mediums/wifi_lan_bwu_handler_test.cc @@ -274,6 +274,33 @@ TEST_F(WifiLanBwuHandlerTest, EXPECT_FALSE(result.has_value()); } +TEST_F(WifiLanBwuHandlerTest, + CreateUpgradedEndpointChannel_RejectFallbackPort0) { + ClientProxy client; + client.AddCancellationFlag(std::string(kEndpointId)); + + BandwidthUpgradeNegotiationFrame::UpgradePathInfo path_info; + path_info.mutable_wifi_lan_socket()->set_ip_address(kIpv4Address); + + // Port 0 + path_info.mutable_wifi_lan_socket()->set_wifi_port(0); + auto result = handler_.CreateUpgradedEndpointChannel( + &client, std::string(kServiceId), std::string(kEndpointId), path_info); + EXPECT_FALSE(result.has_value()); + + // Port > 65535 + path_info.mutable_wifi_lan_socket()->set_wifi_port(65536); + result = handler_.CreateUpgradedEndpointChannel( + &client, std::string(kServiceId), std::string(kEndpointId), path_info); + EXPECT_FALSE(result.has_value()); + + // Port < 0 + path_info.mutable_wifi_lan_socket()->set_wifi_port(-1); + result = handler_.CreateUpgradedEndpointChannel( + &client, std::string(kServiceId), std::string(kEndpointId), path_info); + EXPECT_FALSE(result.has_value()); +} + TEST_F(WifiLanBwuHandlerTest, InitializeUpgradedMediumForEndpoint_Success) { MediumEnvironment::Instance().Start({.use_simulated_clock = true}); ClientProxy client; diff --git a/connections/implementation/offline_frames_validator.cc b/connections/implementation/offline_frames_validator.cc index 20db5702..9c54cf9c 100644 --- a/connections/implementation/offline_frames_validator.cc +++ b/connections/implementation/offline_frames_validator.cc @@ -26,6 +26,7 @@ #include "connections/medium_selector.h" #include "internal/platform/exception.h" #include "internal/platform/logging.h" +#include "internal/platform/service_address.h" namespace nearby { namespace connections { @@ -75,6 +76,11 @@ inline bool WithinRange(int value, int min, int max) { return value >= min && value <= max; } +bool IsValidWifiLanServiceAddress(const ServiceAddress& service_address) { + return !service_address.IsLoopbackAddress() && + !service_address.IsLinkLocalAddress(); +} + Exception EnsureValidConnectionRequestFrame( const ConnectionRequestFrame& frame) { if (frame.endpoint_id().empty()) return {Exception::kInvalidProtocolBuffer}; @@ -234,21 +240,29 @@ Exception EnsureValidBandwidthUpgradeWifiHotspotPathAvailableFrame( !WithinRange(wifi_hotspot_credentials.password().length(), kWifiPasswordSsidMinLength, kWifiPasswordSsidMaxLength)) return {Exception::kInvalidProtocolBuffer}; - if (!wifi_hotspot_credentials.has_gateway() && - wifi_hotspot_credentials.address_candidates_size() == 0) - return {Exception::kInvalidProtocolBuffer}; - const std::regex ip4_pattern(std::string(kIpv4PatternString).c_str()); - if (!wifi_hotspot_credentials.gateway().empty() && - !(std::regex_match(wifi_hotspot_credentials.gateway(), ip4_pattern))) { + + if ((!wifi_hotspot_credentials.has_gateway() || + wifi_hotspot_credentials.gateway().empty()) && + wifi_hotspot_credentials.address_candidates_size() == 0) { return {Exception::kInvalidProtocolBuffer}; } - for (const auto& address_candidate : - wifi_hotspot_credentials.address_candidates()) { - if (!address_candidate.has_ip_address() || !address_candidate.has_port()) { + + const std::regex ip4_pattern(std::string(kIpv4PatternString).c_str()); + if (wifi_hotspot_credentials.has_gateway() && + !wifi_hotspot_credentials.gateway().empty()) { + if (!(std::regex_match(wifi_hotspot_credentials.gateway(), ip4_pattern))) { return {Exception::kInvalidProtocolBuffer}; } - if (address_candidate.ip_address().size() != 4 && - address_candidate.ip_address().size() != 16) { + if (!wifi_hotspot_credentials.has_port() || + !WithinRange(wifi_hotspot_credentials.port(), 1, 65535)) { + return {Exception::kInvalidProtocolBuffer}; + } + } + + for (const auto& address_candidate : + wifi_hotspot_credentials.address_candidates()) { + ServiceAddress service_address; + if (!ServiceAddressFromProto(address_candidate, service_address)) { return {Exception::kInvalidProtocolBuffer}; } } @@ -265,6 +279,29 @@ Exception EnsureValidBandwidthUpgradeWifiLanPathAvailableFrame( return {Exception::kInvalidProtocolBuffer}; } + if (wifi_lan_socket.has_ip_address()) { + location::nearby::connections::ServiceAddress proto; + proto.set_ip_address(wifi_lan_socket.ip_address()); + proto.set_port(wifi_lan_socket.wifi_port()); + ServiceAddress service_address; + if (!ServiceAddressFromProto(proto, service_address)) { + return {Exception::kInvalidProtocolBuffer}; + } + if (!IsValidWifiLanServiceAddress(service_address)) { + return {Exception::kInvalidProtocolBuffer}; + } + } + + for (const auto& address_candidate : wifi_lan_socket.address_candidates()) { + ServiceAddress service_address; + if (!ServiceAddressFromProto(address_candidate, service_address)) { + return {Exception::kInvalidProtocolBuffer}; + } + if (!IsValidWifiLanServiceAddress(service_address)) { + return {Exception::kInvalidProtocolBuffer}; + } + } + // For backwards compatibility reasons, no other fields should be null-checked // for this frame. Parameter checking (eg. must be within this range) is fine. return {Exception::kSuccess}; diff --git a/connections/implementation/offline_frames_validator_test.cc b/connections/implementation/offline_frames_validator_test.cc index 38d573af..59c12271 100644 --- a/connections/implementation/offline_frames_validator_test.cc +++ b/connections/implementation/offline_frames_validator_test.cc @@ -36,6 +36,7 @@ using ::location::nearby::connections::BandwidthUpgradeNegotiationFrame; using ::location::nearby::connections::OfflineFrame; using ::location::nearby::connections::OsInfo; using ::location::nearby::connections::PayloadTransferFrame; +using ::location::nearby::connections::V1Frame; constexpr absl::string_view kEndpointId{"ABC"}; constexpr absl::string_view kEndpointName{"XYZ"}; @@ -122,7 +123,7 @@ TEST_F(OfflineFramesConnectionRequestTest, ValidatesAsFailWithEmptyEndpointIdInConnectionRequestFrame) { connection_info_.local_endpoint_id = ""; std::string bytes = ForConnectionRequestConnections({}, connection_info_); - location::nearby::connections::OfflineFrame frame; + OfflineFrame frame; frame.ParseFromString(bytes); frame.mutable_v1()->mutable_connection_request()->set_endpoint_id(""); ASSERT_TRUE(frame.v1().connection_request().has_endpoint_id()); @@ -680,6 +681,86 @@ TEST(OfflineFramesValidatorTest, EXPECT_FALSE(ret_value.Ok()); } +TEST(OfflineFramesValidatorTest, + ValidateHotspotUpgradeFrameWithLargePortCandidateFails) { + OfflineFrame offline_frame; + + BandwidthUpgradeNegotiationFrame::UpgradePathInfo::WifiHotspotCredentials + credentials; + credentials.set_ssid(kSsid); + credentials.set_password(kPassword); + credentials.set_frequency(kHotspotFrequency); + auto* candidate = credentials.mutable_address_candidates()->Add(); + candidate->set_ip_address(std::string("\xc0\xa8\x00\x01", 4)); + candidate->set_port(70000); + std::string bytes = ForBwuWifiHotspotPathAvailable( + std::move(credentials), kSupportsDisablingEncryption); + offline_frame.ParseFromString(bytes); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateHotspotUpgradeFrameWithFallbackInvalidPortFails) { + OfflineFrame offline_frame; + + BandwidthUpgradeNegotiationFrame::UpgradePathInfo::WifiHotspotCredentials + credentials; + credentials.set_ssid(kSsid); + credentials.set_password(kPassword); + credentials.set_frequency(kHotspotFrequency); + credentials.set_gateway(std::string(kWifiHotspotGateway)); + credentials.set_port(70000); + std::string bytes = ForBwuWifiHotspotPathAvailable( + std::move(credentials), kSupportsDisablingEncryption); + offline_frame.ParseFromString(bytes); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateHotspotUpgradeFrameWithEmptyGatewayAndNoCandidatesFails) { + OfflineFrame offline_frame; + + BandwidthUpgradeNegotiationFrame::UpgradePathInfo::WifiHotspotCredentials + credentials; + credentials.set_ssid(kSsid); + credentials.set_password(kPassword); + credentials.set_frequency(kHotspotFrequency); + credentials.set_gateway(""); + std::string bytes = ForBwuWifiHotspotPathAvailable( + std::move(credentials), kSupportsDisablingEncryption); + offline_frame.ParseFromString(bytes); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateHotspotUpgradeFrameWithNoGatewayAndNoCandidatesFails) { + OfflineFrame offline_frame; + + BandwidthUpgradeNegotiationFrame::UpgradePathInfo::WifiHotspotCredentials + credentials; + credentials.set_ssid(kSsid); + credentials.set_password(kPassword); + credentials.set_frequency(kHotspotFrequency); + // Do not set gateway + // Do not set address candidates + std::string bytes = ForBwuWifiHotspotPathAvailable( + std::move(credentials), kSupportsDisablingEncryption); + offline_frame.ParseFromString(bytes); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + TEST(OfflineFramesValidatorTest, ValidateWifiLanUpgradeFrameWithAddressCandidatesSucceeds) { OfflineFrame offline_frame; @@ -697,6 +778,144 @@ TEST(OfflineFramesValidatorTest, EXPECT_TRUE(ret_value.Ok()); } +TEST(OfflineFramesValidatorTest, + ValidateWifiLanUpgradeFrameWithLoopbackAddressCandidateFails) { + OfflineFrame offline_frame; + std::vector address_candidates = { + {{127, 0, 0, 1}, kPort}, + }; + std::string bytes = ForBwuWifiLanPathAvailable(address_candidates); + offline_frame.ParseFromString(bytes); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateWifiLanUpgradeFrameWithLinkLocalAddressCandidateFails) { + OfflineFrame offline_frame; + std::vector address_candidates = { + {{169, 254, 1, 1}, kPort}, + }; + std::string bytes = ForBwuWifiLanPathAvailable(address_candidates); + offline_frame.ParseFromString(bytes); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateWifiLanUpgradeFrameWithInvalidIpAddressSizeCandidateFails) { + OfflineFrame offline_frame; + std::vector address_candidates = { + {{1, 2, 3}, kPort}, + }; + std::string bytes = ForBwuWifiLanPathAvailable(address_candidates); + offline_frame.ParseFromString(bytes); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateWifiLanUpgradeFrameWithZeroPortCandidateFails) { + OfflineFrame offline_frame; + std::vector address_candidates = { + {{192, 168, 1, 1}, 0}, + }; + std::string bytes = ForBwuWifiLanPathAvailable(address_candidates); + offline_frame.ParseFromString(bytes); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateWifiLanUpgradeFrameWithLargePortCandidateFails) { + OfflineFrame offline_frame; + std::vector address_candidates = { + {{192, 168, 1, 1}, kPort}, + }; + std::string bytes = ForBwuWifiLanPathAvailable(address_candidates); + offline_frame.ParseFromString(bytes); + + auto* negotiation = + offline_frame.mutable_v1()->mutable_bandwidth_upgrade_negotiation(); + auto* wifi_lan_socket = + negotiation->mutable_upgrade_path_info()->mutable_wifi_lan_socket(); + if (wifi_lan_socket->address_candidates_size() > 0) { + wifi_lan_socket->mutable_address_candidates(0)->set_port(70000); + } + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateWifiLanUpgradeFrameWithFallbackLoopbackAddressFails) { + OfflineFrame offline_frame; + offline_frame.set_version(OfflineFrame::V1); + auto* v1_frame = offline_frame.mutable_v1(); + v1_frame->set_type(V1Frame::BANDWIDTH_UPGRADE_NEGOTIATION); + auto* negotiation = v1_frame->mutable_bandwidth_upgrade_negotiation(); + negotiation->set_event_type( + BandwidthUpgradeNegotiationFrame::UPGRADE_PATH_AVAILABLE); + auto* upgrade_path_info = negotiation->mutable_upgrade_path_info(); + upgrade_path_info->set_medium(UpgradePathInfo::WIFI_LAN); + auto* wifi_lan_socket = upgrade_path_info->mutable_wifi_lan_socket(); + wifi_lan_socket->set_ip_address(std::string({127, 0, 0, 1})); + wifi_lan_socket->set_wifi_port(kPort); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateWifiLanUpgradeFrameWithFallbackLinkLocalAddressFails) { + OfflineFrame offline_frame; + offline_frame.set_version(OfflineFrame::V1); + auto* v1_frame = offline_frame.mutable_v1(); + v1_frame->set_type(V1Frame::BANDWIDTH_UPGRADE_NEGOTIATION); + auto* negotiation = v1_frame->mutable_bandwidth_upgrade_negotiation(); + negotiation->set_event_type( + BandwidthUpgradeNegotiationFrame::UPGRADE_PATH_AVAILABLE); + auto* upgrade_path_info = negotiation->mutable_upgrade_path_info(); + upgrade_path_info->set_medium(UpgradePathInfo::WIFI_LAN); + auto* wifi_lan_socket = upgrade_path_info->mutable_wifi_lan_socket(); + wifi_lan_socket->set_ip_address(std::string({169, 254, 1, 1})); + wifi_lan_socket->set_wifi_port(kPort); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + +TEST(OfflineFramesValidatorTest, + ValidateWifiLanUpgradeFrameWithFallbackInvalidPortFails) { + OfflineFrame offline_frame; + offline_frame.set_version(OfflineFrame::V1); + auto* v1_frame = offline_frame.mutable_v1(); + v1_frame->set_type(V1Frame::BANDWIDTH_UPGRADE_NEGOTIATION); + auto* negotiation = v1_frame->mutable_bandwidth_upgrade_negotiation(); + negotiation->set_event_type( + BandwidthUpgradeNegotiationFrame::UPGRADE_PATH_AVAILABLE); + auto* upgrade_path_info = negotiation->mutable_upgrade_path_info(); + upgrade_path_info->set_medium(UpgradePathInfo::WIFI_LAN); + auto* wifi_lan_socket = upgrade_path_info->mutable_wifi_lan_socket(); + wifi_lan_socket->set_ip_address(std::string({192, 168, 1, 1})); + wifi_lan_socket->set_wifi_port(70000); + + auto ret_value = EnsureValidOfflineFrame(offline_frame); + + EXPECT_FALSE(ret_value.Ok()); +} + TEST(OfflineFramesValidatorTest, ValidatesAsFailWithNullBandwidthUpgradeNegotiationFrame) { OfflineFrame offline_frame; diff --git a/internal/platform/service_address.cc b/internal/platform/service_address.cc index d29d0a3f..f2bf54d1 100644 --- a/internal/platform/service_address.cc +++ b/internal/platform/service_address.cc @@ -37,9 +37,9 @@ void ServiceAddressToProto( bool ServiceAddressFromProto( const location::nearby::connections::ServiceAddress& proto, ServiceAddress& service_address) { - // Address must be either 4 or 16 bytes and port must be set. + // Address must be either 4 or 16 bytes and port must be valid (1 to 65535). if ((proto.ip_address().size() != 16 && proto.ip_address().size() != 4) || - proto.port() == 0) { + proto.port() <= 0 || proto.port() > 65535) { return false; } service_address.address = {proto.ip_address().begin(), diff --git a/internal/platform/service_address_test.cc b/internal/platform/service_address_test.cc index b7aed877..c4fa2285 100644 --- a/internal/platform/service_address_test.cc +++ b/internal/platform/service_address_test.cc @@ -81,8 +81,18 @@ TEST(ServiceAddressTest, ServiceAddressFromProtoInvalidAddress) { TEST(ServiceAddressTest, ServiceAddressFromProtoInvalidPort) { ProtoServiceAddress proto; proto.set_ip_address(std::string("\x7f\0\0\1", 4)); - proto.set_port(0); ServiceAddress service_address; + + // Port 0 + proto.set_port(0); + EXPECT_FALSE(ServiceAddressFromProto(proto, service_address)); + + // Port > 65535 + proto.set_port(65536); + EXPECT_FALSE(ServiceAddressFromProto(proto, service_address)); + + // Port < 0 + proto.set_port(-1); EXPECT_FALSE(ServiceAddressFromProto(proto, service_address)); }