diff --git a/assets/js/app.ts b/assets/js/app.ts index a1f79299..9f1b85ab 100644 --- a/assets/js/app.ts +++ b/assets/js/app.ts @@ -445,7 +445,11 @@ declare global { } } -window.liveSocket = liveSocket +// Only expose for debugging in non-production environments +declare global { interface Window { NODE_ENV?: string } } +if (typeof window !== "undefined" && window.NODE_ENV !== "production") { + ;(window as any).liveSocket = liveSocket +} // The lines below enable quality of life phoenix_live_reload // development features: diff --git a/bugs.md b/bugs.md index 79227f32..a3dd296e 100644 --- a/bugs.md +++ b/bugs.md @@ -165,17 +165,6 @@ --- -### L3. IPv4-Mapped IPv6 Not Handled in Whitelist - -**File:** `lib/towerops/security/brute_force.ex:36-49` - -**Severity:** LOW — `::ffff:192.168.1.1` doesn't match whitelist entry `192.168.1.1` - -**Description:** Exact string matching means IPv4-mapped IPv6 addresses bypass whitelist matching. - -**Fix:** Normalize IPs via `:inet.parse_address/1` before comparing. - ---- ### L4. No Auth on PromEx Metrics Port (Documentation Gap) @@ -189,41 +178,8 @@ --- -### L7. Device Capacity Operations Missing Org Scoping -**File:** `lib/towerops_web/live/device_live/show.ex:728-757` -**Severity:** LOW — `interface_id` from params not verified to belong to user's org - -**Description:** `set_capacity` handler uses the `interface_id` from client params with no org membership verification. - -**Fix:** Verify interface belongs to a device in the user's org. - ---- - -### L12. `window.liveSocket` Exposed Globally - -**File:** `assets/js/app.ts:423` - -**Severity:** LOW — CSRF token accessible via `liveSocket.socket.params._csrf_token` - -**Description:** Exposing LiveSocket globally makes CSRF token trivially accessible to any XSS. - -**Fix:** Guard behind `NODE_ENV !== "production"`. - ---- - -### L13. Overly Broad Changeset in Alert Updates - -**File:** `lib/towerops/alerts.ex:391-405` - -**Severity:** LOW — `Alert.changeset/2` casts 17 fields for a partial update - -**Description:** Using the full changeset for simple field updates (`resolved_at`) is fragile. Extra keys could overwrite system fields. - -**Fix:** Use a dedicated changeset or `Ecto.Changeset.change/2`. - ---- ### L14. `list_checks` No Pagination diff --git a/lib/towerops/alerts.ex b/lib/towerops/alerts.ex index 2a0df6f4..72bfe0fc 100644 --- a/lib/towerops/alerts.ex +++ b/lib/towerops/alerts.ex @@ -423,7 +423,7 @@ defmodule Towerops.Alerts do # Performs the database update for resolving an alert defp do_resolve_alert(alert) do alert - |> Alert.changeset(%{resolved_at: Towerops.Time.now()}) + |> Ecto.Changeset.change(%{resolved_at: Towerops.Time.now()}) |> Repo.update() end @@ -433,7 +433,7 @@ defmodule Towerops.Alerts do attrs = if user_id, do: Map.put(attrs, :acknowledged_by_id, user_id), else: attrs alert - |> Alert.changeset(attrs) + |> Ecto.Changeset.change(attrs) |> Repo.update() end @@ -442,7 +442,7 @@ defmodule Towerops.Alerts do """ def store_gaiia_impact(%Alert{} = alert, impact_data) when is_map(impact_data) do alert - |> Alert.changeset(%{gaiia_impact: impact_data}) + |> Ecto.Changeset.change(%{gaiia_impact: impact_data}) |> Repo.update() end diff --git a/lib/towerops/security/brute_force.ex b/lib/towerops/security/brute_force.ex index 03f3c9c9..bb0673b4 100644 --- a/lib/towerops/security/brute_force.ex +++ b/lib/towerops/security/brute_force.ex @@ -34,20 +34,41 @@ defmodule Towerops.Security.BruteForce do end defp ip_matches_entry?(ip_address, entry) do + case parse_ip(ip_address) do + {:ok, parsed_ip} -> entry_matches_parsed_ip?(entry, parsed_ip) + _ -> false + end + end + + defp entry_matches_parsed_ip?(entry, parsed_ip) do if String.contains?(entry.ip_or_cidr, "/") do - # CIDR range - need to parse both the range and IP address - with {:ok, range} <- InetCidr.parse_cidr(entry.ip_or_cidr), - {:ok, addr} <- :inet.parse_address(String.to_charlist(ip_address)) do - InetCidr.contains?(range, addr) - else + case InetCidr.parse_cidr(entry.ip_or_cidr) do + {:ok, range} -> InetCidr.contains?(range, parsed_ip) _ -> false end else - # Exact IP match - entry.ip_or_cidr == ip_address + case parse_ip(entry.ip_or_cidr) do + {:ok, parsed_entry} -> parsed_entry == parsed_ip + _ -> false + end end end + defp parse_ip(addr) when is_binary(addr) do + case :inet.parse_address(String.to_charlist(addr)) do + {:ok, ip} -> {:ok, normalize_ipv4_mapped(ip)} + error -> error + end + end + + # Normalize IPv4-mapped IPv6 addresses (::ffff:x.x.x.x) to IPv4 tuples + # so they match whitelist entries written as plain IPv4. + defp normalize_ipv4_mapped({0, 0, 0, 0, 0, 65_535, a, b}) do + {div(a, 256), rem(a, 256), div(b, 256), rem(b, 256)} + end + + defp normalize_ipv4_mapped(ip), do: ip + @doc """ Lists all whitelisted IPs and CIDR ranges. """ diff --git a/lib/towerops_web/live/device_live/show.ex b/lib/towerops_web/live/device_live/show.ex index 854359fc..5da16582 100644 --- a/lib/towerops_web/live/device_live/show.ex +++ b/lib/towerops_web/live/device_live/show.ex @@ -726,33 +726,40 @@ defmodule ToweropsWeb.DeviceLive.Show do @impl true def handle_event("set_capacity", %{"interface_id" => id, "capacity_mbps" => mbps_str}, socket) do - case Float.parse(mbps_str) do - {mbps, _} when mbps > 0 -> - bps = round(mbps * 1_000_000) + org_id = socket.assigns.current_scope.organization.id - case Snmp.set_manual_capacity(id, bps) do - {:ok, _} -> - socket = reload_snmp_and_ports(socket) - {:noreply, put_flash(socket, :info, t("Capacity updated"))} + with {:ok, mbps} <- parse_capacity(mbps_str), + true <- interface_belongs_to_org?(id, org_id) do + bps = round(mbps * 1_000_000) - {:error, _} -> - {:noreply, put_flash(socket, :error, t("Failed to update capacity"))} - end + case Snmp.set_manual_capacity(id, bps) do + {:ok, _} -> + {:noreply, put_flash(reload_snmp_and_ports(socket), :info, t("Capacity updated"))} - _ -> - {:noreply, put_flash(socket, :error, t("Invalid capacity value"))} + {:error, _} -> + {:noreply, put_flash(socket, :error, t("Failed to update capacity"))} + end + else + {:error, :invalid} -> {:noreply, put_flash(socket, :error, t("Invalid capacity value"))} + false -> {:noreply, put_flash(socket, :error, t("Interface not found"))} end end @impl true def handle_event("clear_capacity", %{"interface_id" => id}, socket) do - case Snmp.clear_manual_capacity(id) do - {:ok, _} -> - socket = reload_snmp_and_ports(socket) - {:noreply, put_flash(socket, :info, t("Capacity cleared"))} + org_id = socket.assigns.current_scope.organization.id - {:error, _} -> - {:noreply, put_flash(socket, :error, t("Failed to clear capacity"))} + if interface_belongs_to_org?(id, org_id) do + case Snmp.clear_manual_capacity(id) do + {:ok, _} -> + socket = reload_snmp_and_ports(socket) + {:noreply, put_flash(socket, :info, t("Capacity cleared"))} + + {:error, _} -> + {:noreply, put_flash(socket, :error, t("Failed to clear capacity"))} + end + else + {:noreply, put_flash(socket, :error, t("Interface not found"))} end end @@ -922,6 +929,32 @@ defmodule ToweropsWeb.DeviceLive.Show do end end + defp parse_capacity(mbps_str) do + case Float.parse(mbps_str) do + {mbps, _} when mbps > 0 -> {:ok, mbps} + _ -> {:error, :invalid} + end + end + + defp interface_belongs_to_org?(interface_id, org_id) do + {:ok, iid} = Ecto.UUID.dump(interface_id) + {:ok, oid} = Ecto.UUID.dump(org_id) + + result = + Towerops.Repo.query!( + """ + SELECT 1 FROM snmp_interfaces i + JOIN snmp_devices sd ON sd.id = i.snmp_device_id + JOIN devices d ON d.id = sd.device_id + WHERE i.id = $1 AND d.organization_id = $2 + LIMIT 1 + """, + [iid, oid] + ) + + result.num_rows > 0 + end + defp reload_snmp_and_ports(socket) do device_id = socket.assigns.device.id snmp_data = DataLoaders.load_snmp_data(device_id)