fix: L3, L7, L12, L13 — IP whitelist normalization, org scoping, liveSocket guard, changeset safety
- L3: Normalize IPv4-mapped IPv6 addresses (::ffff:x.x.x.x) to IPv4 tuples when matching against IP whitelist entries - L7: Verify interface belongs to current org before set/clear capacity - L12: Guard window.liveSocket behind NODE_ENV check for production safety - L13: Replace Alert.changeset with Ecto.Changeset.change for simple field updates (resolved_at, acknowledged_at, gaiia_impact) to prevent accidental overwrites from the 17-field cast
This commit is contained in:
parent
cec8eabcd8
commit
c473f472c7
5 changed files with 87 additions and 73 deletions
|
|
@ -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:
|
||||
|
|
|
|||
44
bugs.md
44
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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue