Address review comments on port validation.

PiperOrigin-RevId: 946184592

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<char> 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 "

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<OfflineFrame> upgrade_frame;
 
   auto handler_1 = std::make_unique<WifiHotspotBwuHandler>(
-      &mediums_HS_ap.GetWifiHotspot(), [&](ClientProxy* client,
-                         std::unique_ptr<BwuHandler::IncomingSocketConnection>
-                             mutable_connection) {
+      &mediums_HS_ap.GetWifiHotspot(),
+      [&](ClientProxy* client,
+          std::unique_ptr<BwuHandler::IncomingSocketConnection>
+              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

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 ("

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;

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};

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<ServiceAddress> 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<ServiceAddress> 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<ServiceAddress> 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<ServiceAddress> 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<ServiceAddress> 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;

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(),

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));
 }