hardening playerspec type checking

CHANGELOG.md

@@ -1,4 +1,4 @@
-### 1.7.37 (build 22148, api 9, 2024-12-28)
+### 1.7.37 (build 22150, api 9, 2024-12-28)
 - Bumping api version to 9. As you'll see below, there's some UI changes that
   will require a bit of work for any UI mods to adapt to. If your mods don't
   touch UI stuff at all you can simply bump your api version and call it a day.

src/assets/ba_data/python/baenv.py

@@ -53,7 +53,7 @@
 
 # Build number and version of the ballistica binary we expect to be
 # using.
-TARGET_BALLISTICA_BUILD = 22148
+TARGET_BALLISTICA_BUILD = 22150
 TARGET_BALLISTICA_VERSION = '1.7.37'
 
 

src/ballistica/scene_v1/connection/connection_to_client.cc

@@ -191,13 +191,27 @@ void ConnectionToClient::HandleGamePacket(const std::vector<uint8_t>& data) {
         string_buffer[string_buffer.size() - 1] = 0;
         set_peer_spec(PlayerSpec(&(string_buffer[0])));
       }
+
+      // If they sent us a garbage player-spec, kick them right out.
+      if (!peer_spec().valid()) {
+        g_core->Log(LogName::kBaNetworking, LogLevel::kDebug, [] {
+          return std::string(
+              "Rejecting client for submitting invalid player-spec.");
+        });
+        Error("");
+        return;
+      }
+
       // FIXME: We should maybe set some sort of 'pending' peer-spec
       //  and fetch their actual info from the master-server.
       //  (or at least make that an option for internet servers)
 
       // Compare this against our blocked specs.. if there's a match, reject
       // them.
       if (appmode->IsPlayerBanned(peer_spec())) {
+        g_core->Log(LogName::kBaNetworking, LogLevel::kDebug, [] {
+          return std::string("Rejecting join attempt by banned player.");
+        });
         Error("");
         return;
       }

src/ballistica/scene_v1/support/player_spec.cc

@@ -18,32 +18,36 @@ PlayerSpec::PlayerSpec(const std::string& s) {
   cJSON* root_obj = cJSON_Parse(s.c_str());
   bool success = false;
   if (root_obj) {
-    cJSON* name_obj = cJSON_GetObjectItem(root_obj, "n");
-    cJSON* short_name_obj = cJSON_GetObjectItem(root_obj, "sn");
-    cJSON* account_obj = cJSON_GetObjectItem(root_obj, "a");
-    if (name_obj && short_name_obj && account_obj) {
-      name_ = Utils::GetValidUTF8(name_obj->valuestring, "psps");
-      short_name_ = Utils::GetValidUTF8(short_name_obj->valuestring, "psps2");
-
-      // Account type may technically be something we don't recognize,
-      // but that's ok.. it'll just be 'invalid' to us in that case
-      if (g_base->HaveClassic()) {
-        v1_account_type_ = g_base->classic()->GetV1AccountTypeFromString(
-            account_obj->valuestring);
-        // classic::V1Account::AccountTypeFromString(account_obj->valuestring);
-      } else {
-        v1_account_type_ = 0;  // kInvalid.
+    if (cJSON_IsObject(root_obj)) {
+      cJSON* name_obj = cJSON_GetObjectItem(root_obj, "n");
+      cJSON* short_name_obj = cJSON_GetObjectItem(root_obj, "sn");
+      cJSON* account_obj = cJSON_GetObjectItem(root_obj, "a");
+      if (name_obj && short_name_obj && account_obj && cJSON_IsString(name_obj)
+          && cJSON_IsString(short_name_obj) && cJSON_IsString(account_obj)) {
+        name_ = Utils::GetValidUTF8(name_obj->valuestring, "psps");
+        short_name_ = Utils::GetValidUTF8(short_name_obj->valuestring, "psps2");
+
+        // Account type may technically be something we don't recognize,
+        // but that's ok.. it'll just be 'invalid' to us in that case
+        if (g_base->HaveClassic()) {
+          v1_account_type_ = g_base->classic()->GetV1AccountTypeFromString(
+              account_obj->valuestring);
+        } else {
+          v1_account_type_ = 0;  // kInvalid.
+        }
+        success = true;
       }
-      success = true;
     }
     cJSON_Delete(root_obj);
   }
   if (!success) {
-    g_core->Log(LogName::kBa, LogLevel::kError,
+    valid_ = false;
+
+    // Only log this once in case it is used as an attack.
+    BA_LOG_ONCE(LogName::kBa, LogLevel::kError,
                 "Error creating PlayerSpec from string: '" + s + "'");
     name_ = "<error>";
-    short_name_ = "";
-    // account_type_ = classic::V1AccountType::kInvalid;
+    short_name_ = "<error>";
     v1_account_type_ = 0;  // kInvalid.
   }
 }
@@ -54,7 +58,6 @@ auto PlayerSpec::GetDisplayString() const -> std::string {
            + name_;
   }
   return name_;
-  // return classic::V1Account::AccountTypeToIconString(account_type_) + name_;
 }
 
 auto PlayerSpec::GetShortName() const -> std::string {
@@ -76,7 +79,6 @@ auto PlayerSpec::GetSpecString() const -> std::string {
   cJSON_AddStringToObject(root, "n", name_.c_str());
   cJSON_AddStringToObject(
       root, "a",
-      // classic::V1Account::AccountTypeToString(account_type_).c_str()
       g_base->HaveClassic()
           ? g_base->classic()->V1AccountTypeToString(v1_account_type_).c_str()
           : "");

src/ballistica/scene_v1/support/player_spec.h

@@ -46,10 +46,13 @@ class PlayerSpec {
   /// party hosts, etc.
   static auto GetDummyPlayerSpec(const std::string& name) -> PlayerSpec;
 
+  auto valid() const { return valid_; }
+
  private:
   std::string name_;
   std::string short_name_;
   int v1_account_type_{};
+  bool valid_{true};
 };
 
 }  // namespace ballistica::scene_v1

src/ballistica/shared/ballistica.cc

@@ -39,7 +39,7 @@ auto main(int argc, char** argv) -> int {
 namespace ballistica {
 
 // These are set automatically via script; don't modify them here.
-const int kEngineBuildNumber = 22148;
+const int kEngineBuildNumber = 22150;
 const char* kEngineVersion = "1.7.37";
 const int kEngineApiVersion = 9;