Reject invalid firewall configuration and preserve policy on reload failure

src/config_parser.c

@@ -201,12 +201,16 @@ static int add_route_entry(struct CFG_ROUTE_ENTRY **list, const char *subnet_str
     return 0;
 }
 
+/* Разрешение добавляется только после проверки всей строки; ошибочный порт не становится wildcard. */
 static int parse_firewall_rule(const char *rule_str, struct global_config *global) {
-    if (!rule_str || !global) return -1;
+    if (!rule_str || !global) { DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Firewall: missing rule/config"); return -1; }
 
     char rule_copy[64];
-    strncpy(rule_copy, rule_str, sizeof(rule_copy) - 1);
-    rule_copy[sizeof(rule_copy) - 1] = '\0';
+    if (strlen(rule_str) >= sizeof(rule_copy)) {
+        DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Firewall: rule is too long: %s", rule_str);
+        return -1;
+    }
+    strcpy(rule_copy, rule_str);
     config_ini_trim(rule_copy);
 
     if (strcasecmp(rule_copy, "all") == 0) {
@@ -219,8 +223,15 @@ static int parse_firewall_rule(const char *rule_str, struct global_config *globa
     char *port_str = strchr(rule_copy, ':');
     uint16_t port = 0;
     if (port_str) {
-        *port_str = '\0';
-        port = (uint16_t)atoi(port_str + 1);
+        *port_str++ = '\0';
+        char *end = NULL;
+        errno = 0;
+        unsigned long parsed = strtoul(port_str, &end, 10);
+        if (!isdigit((unsigned char)*port_str) || errno || *end || parsed > UINT16_MAX) {
+            DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Firewall: invalid port in rule: %s", rule_str);
+            return -1;
+        }
+        port = (uint16_t)parsed;
     }
 
     // Parse IP to uint32_t (host order)
@@ -234,6 +245,10 @@ static int parse_firewall_rule(const char *rule_str, struct global_config *globa
 
     // Reallocate array if needed
     if ((global->firewall_rule_count % INITIAL_ARRAY_CAPACITY) == 0) {
+        if ((uint32_t)global->firewall_rule_count > UINT32_MAX / sizeof(struct CFG_FIREWALL_RULE) - INITIAL_ARRAY_CAPACITY) {
+            DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Firewall: too many rules");
+            return -1;
+        }
         int new_cap = global->firewall_rule_count + INITIAL_ARRAY_CAPACITY;
         struct CFG_FIREWALL_RULE *new_rules = u_realloc(global->firewall_rules,
             new_cap * sizeof(struct CFG_FIREWALL_RULE));
@@ -247,7 +262,6 @@ static int parse_firewall_rule(const char *rule_str, struct global_config *globa
     struct CFG_FIREWALL_RULE *rule = &global->firewall_rules[global->firewall_rule_count];
     rule->ip = ip;
     rule->port = port;
-    rule->bypass = 0;
     global->firewall_rule_count++;
 
     DEBUG_DEBUG(DEBUG_CATEGORY_CONFIG, "Added firewall rule: %s:%d", rule_str, port);
@@ -1060,7 +1074,11 @@ struct utun_config* parse_config_from_buf(const char *buf, size_t len, const cha
             continue;
         }
         char key[MAX_LINE_LEN], value[MAX_LINE_LEN];
-        if (config_ini_key_value(trimmed, key, sizeof(key), value, sizeof(value)) < 0) { DEBUG_WARN(DEBUG_CATEGORY_CONFIG, "%s:%d: Invalid key=value format", filename, line_num); continue; }
+        if (config_ini_key_value(trimmed, key, sizeof(key), value, sizeof(value)) < 0) {
+            DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Invalid key=value format", filename, line_num);
+            if (cur_section == SECTION_FIREWALL) goto error;
+            continue;
+        }
         switch (cur_section) {
             case SECTION_GLOBAL: parse_global(key, value, &cfg->global, filename, line_num); break;
             case SECTION_SERVER: if (cur_server) parse_server(key, value, cur_server, filename, line_num); break;
@@ -1073,7 +1091,16 @@ struct utun_config* parse_config_from_buf(const char *buf, size_t len, const cha
                     strncpy(cfg->global.debug_levels.level[index], value, 15); cfg->global.debug_levels.level[index][15] = '\0';
                 } else DEBUG_WARN(DEBUG_CATEGORY_CONFIG, "%s:%d: Too many debug categories, ignored %s=%s", filename, line_num, key, value);
                 break;
-            case SECTION_FIREWALL: if (strcmp(key, "allow") == 0) parse_firewall_rule(value, &cfg->global); else DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Unknown firewall option '%s'", filename, line_num, key); break;
+            case SECTION_FIREWALL:
+                if (strcmp(key, "allow") != 0) {
+                    DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Unknown firewall option '%s'", filename, line_num, key);
+                    goto error;
+                }
+                if (parse_firewall_rule(value, &cfg->global) < 0) {
+                    DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Invalid firewall rule allow=%s", filename, line_num, value);
+                    goto error;
+                }
+                break;
             case SECTION_CONTROL: parse_control(key, value, &cfg->global, filename, line_num); break;
             case SECTION_ALLOWED_KEYS: if (strcmp(key, "allow_all") == 0) cfg->global.allowed_keys_allow_all = strcasecmp(value, "yes") == 0 || strcasecmp(value, "1") == 0 || strcasecmp(value, "true") == 0; else if (strcmp(key, "key") == 0) { struct CFG_ALLOWED_KEY *nk = u_calloc(1, sizeof(struct CFG_ALLOWED_KEY)); if (nk && strlen(value) == SC_PUBKEY_SIZE * 2 && config_ini_hex(value, nk->key_bin, SC_PUBKEY_SIZE) == 0) { nk->next = cfg->global.allowed_keys; cfg->global.allowed_keys = nk; cfg->global.allowed_keys_count++; } else u_free(nk); } else DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Unknown allowed_keys option '%s'", filename, line_num, key); break;
             case SECTION_NAT: cfg->global.nat_enabled = 1; parse_nat(key, value, &cfg->global, filename, line_num); break;

src/utun_instance.c

@@ -996,6 +996,12 @@ struct UTUN_INSTANCE *utun_instance_reload(struct UTUN_INSTANCE *instance, struc
         DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Failed to parse new config");
         return instance;
     }
+    struct firewall_ctx next_fw = {0};
+    if (fw_load_rules(&next_fw, &new_config->global) < 0) {
+        DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Reload rejected: cannot prepare firewall; previous configuration retained");
+        free_config(new_config);
+        return instance;
+    }
 
     // Reset + apply debug categories from [debug] section (always, for partial and full paths).
     // Mirrors utun_instance_create exactly. Reset ensures removed categories fall back to global.
@@ -1022,6 +1028,7 @@ struct UTUN_INSTANCE *utun_instance_reload(struct UTUN_INSTANCE *instance, struc
     }
     if (needs_full) {
         DEBUG_INFO(DEBUG_CATEGORY_CONFIG, "Incompatible global - full reload");
+        fw_free(&next_fw);
         utun_instance_destroy(instance);
         struct UTUN_INSTANCE *new_instance = utun_instance_create(ua, config_file);
         if (!new_instance || utun_instance_init(new_instance) < 0) {
@@ -1102,8 +1109,8 @@ struct UTUN_INSTANCE *utun_instance_reload(struct UTUN_INSTANCE *instance, struc
         DEBUG_INFO(DEBUG_CATEGORY_CONFIG, "Networks reloaded");
     }
     fw_free(&instance->fw);
-    fw_init(&instance->fw);
-    fw_load_rules(&instance->fw, &new_config->global);
+    instance->fw = next_fw;
+    DEBUG_INFO(DEBUG_CATEGORY_CONFIG, "Firewall reload applied: rules=%d bypass_all=%d", instance->fw.count, instance->fw.bypass_all);
     // routing_destroy(instance);  // commented per request - BGP self-restarts on conn events
     // routing_create(instance);   // commented per request
     stun_nat_destroy(instance);