SheWhoReignsSupreme/EveCogs
1
0
Fork 0
Code Issues 4 Pull requests Projects Releases Packages Wiki Activity Actions

Fix dashboard security issues: path traversal, OAuth CSRF, exposed bind, stale cache

Browse source 
- _serve_spa built file paths directly from the URL with no containment
  check, on a route reachable with zero authentication. Verified the exact
  exploit (percent-encoded traversal) reproduces path escape, then fixed by
  resolving the path and requiring it stay under WEB_DIR before ever
  touching the filesystem.
- OAuth state was generated and handed to the client but never bound
  server-side or checked on callback — classic login CSRF/session fixation.
  Now bound to a short-lived httponly cookie set during login and verified
  (constant-time) on callback.
- Default bind changed from 0.0.0.0 to 127.0.0.1 — loading this cog with
  zero config shouldn't expose a web server on every interface before the
  owner has even run setup. `[p]evedash host 0.0.0.0` still opts back in.
- _check_guild_access and the callback's guild-membership check relied
  solely on the member cache, which can wrongly deny a real admin if the
  Members intent hasn't warmed for that guild yet. Both now fall back to a
  live fetch on cache miss (per-guild for the single-guild endpoints,
  bot-wide only in the rare one-time login path — deliberately not done in
  the bulk guild-list endpoint, where that fallback would mean an API call
  per guild on every page load).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rosie Val 2026-07-29 00:08:12 +01:00
parent 5e41658d7d
commit a9cdef8a1b
2 changed files with 67 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
dashboard/dashboard.py
View file

@ -51,7 +51,12 @@ class EveDash(commands.Cog):
force_registration=True,
)
self.config.register_global(
host="0.0.0.0",
# Localhost-only by default — loading this cog with zero config
# shouldn't immediately expose an unauthenticated-until-login web
# server on every network interface. Owners who want it reachable
# externally (e.g. no reverse proxy) can opt in with
# `[p]evedash host 0.0.0.0`.
host="127.0.0.1",
port=42356,
client_id=None,
client_secret=None,

72
dashboard/server.py
View file

@ -17,6 +17,7 @@ import time
import typing
import aiohttp
import discord
from aiohttp import web
if typing.TYPE_CHECKING:
@ -259,13 +260,30 @@ class DashboardServer:
f"&scope=identify+guilds"
f"&state={state}"
)
return _json({"url": url, "state": state})
resp = _json({"url": url, "state": state})
# Bind this state to the browser that requested it via a short-lived
# cookie, so the callback can confirm the redirect actually belongs
# to a login flow we started — otherwise an attacker's own
# authorization code could be handed to a victim's browser and get
# bound to the victim's session (login CSRF / session fixation).
resp.set_cookie(
"oauth_state", state,
max_age=600,
httponly=True,
samesite="Lax",
)
return resp
async def _auth_callback(self, request: web.Request) -> web.Response:
code = request.query.get("code")
if not code:
return _err("Missing authorization code")
state = request.query.get("state")
expected_state = request.cookies.get("oauth_state")
if not state or not expected_state or not hmac.compare_digest(state, expected_state):
return _err("Invalid or missing state parameter", 400)
client_id = await self.cog.config.client_id()
client_secret = await self.cog.config.client_secret()
redirect_uri = await self.cog.config.redirect_uri()
@ -301,6 +319,17 @@ class DashboardServer:
# Check access: must be bot owner or share a guild
is_owner = user_id in self.bot.owner_ids
has_guild = any(g.get_member(user_id) for g in self.bot.guilds)
if not is_owner and not has_guild:
# Cache miss rather than genuinely no shared guilds? This only
# runs once per login (not a hot path), so it's worth confirming
# with a live check before denying a real user access.
for g in self.bot.guilds:
try:
if await g.fetch_member(user_id):
has_guild = True
break
except (discord.NotFound, discord.HTTPException):
continue
if not is_owner and not has_guild:
return _err("You don't share any servers with this bot", 403)
@ -322,6 +351,7 @@ class DashboardServer:
httponly=True,
samesite="Lax",
)
resp.del_cookie("oauth_state")
return resp
async def _auth_me(self, request: web.Request) -> web.Response:
@ -437,7 +467,7 @@ class DashboardServer:
if not guild:
return _err("Guild not found", 404)
if not self._check_guild_access(request, guild):
if not await self._check_guild_access(request, guild):
return _err("Forbidden", 403)
return _json({
@ -462,7 +492,7 @@ class DashboardServer:
guild = self._get_guild(request)
if not guild:
return _err("Guild not found", 404)
if not self._check_guild_access(request, guild):
if not await self._check_guild_access(request, guild):
return _err("Forbidden", 403)
channels = []
@ -480,7 +510,7 @@ class DashboardServer:
guild = self._get_guild(request)
if not guild:
return _err("Guild not found", 404)
if not self._check_guild_access(request, guild):
if not await self._check_guild_access(request, guild):
return _err("Forbidden", 403)
roles = []
@ -503,7 +533,7 @@ class DashboardServer:
guild = self._get_guild(request)
if not guild:
return _err("Guild not found", 404)
if not self._check_guild_access(request, guild):
if not await self._check_guild_access(request, guild):
return _err("Forbidden", 403)
prefixes = await self.bot.get_valid_prefixes(guild)
@ -534,7 +564,7 @@ class DashboardServer:
guild = self._get_guild(request)
if not guild:
return _err("Guild not found", 404)
if not self._check_guild_access(request, guild):
if not await self._check_guild_access(request, guild):
return _err("Forbidden", 403)
data = await request.json()
@ -568,7 +598,7 @@ class DashboardServer:
guild = self._get_guild(request)
if not guild:
return _err("Guild not found", 404)
if not self._check_guild_access(request, guild):
if not await self._check_guild_access(request, guild):
return _err("Forbidden", 403)
disabled_cmds = await self.bot._config.guild(guild).disabled_commands()
@ -600,7 +630,7 @@ class DashboardServer:
guild = self._get_guild(request)
if not guild:
return _err("Guild not found", 404)
if not self._check_guild_access(request, guild):
if not await self._check_guild_access(request, guild):
return _err("Forbidden", 403)
cmd_name = request.match_info["command"]
@ -693,7 +723,7 @@ class DashboardServer:
guild = self._get_guild(request)
if not guild:
return _err("Guild not found", 404)
if not self._check_guild_access(request, guild):
if not await self._check_guild_access(request, guild):
return _err("Forbidden", 403)
tp_info = self.cog.third_parties.get_third_parties_info()
@ -703,7 +733,7 @@ class DashboardServer:
guild = self._get_guild(request)
if not guild:
return _err("Guild not found", 404)
if not self._check_guild_access(request, guild):
if not await self._check_guild_access(request, guild):
return _err("Forbidden", 403)
cog_name = request.match_info["cog_name"]
@ -851,7 +881,18 @@ class DashboardServer:
async def _serve_spa(self, request: web.Request) -> web.Response:
path = request.match_info.get("path", "")
file_path = WEB_DIR / path
# Resolve and verify the final path stays inside WEB_DIR before ever
# touching the filesystem — rejects `../`, percent-encoded traversal
# (e.g. `..%2f`), and symlink escapes alike. This route is reachable
# with no authentication (see the auth middleware's path allowlist),
# so this check is the only thing standing between it and arbitrary
# file read on the host.
try:
file_path = (WEB_DIR / path).resolve()
file_path.relative_to(WEB_DIR.resolve())
except (ValueError, OSError):
return _err("Not found", 404)
# Serve actual files if they exist
if file_path.is_file():
@ -872,11 +913,18 @@ class DashboardServer:
except (KeyError, ValueError):
return None
def _check_guild_access(self, request: web.Request, guild) -> bool:
async def _check_guild_access(self, request: web.Request, guild) -> bool:
if request["is_owner"]:
return True
user_id = request["user_id"]
member = guild.get_member(user_id)
if not member:
# Cache miss (e.g. Members intent not warmed for this guild yet)
# shouldn't wrongly deny a real admin — fall back to a live fetch.
# Only done for single-guild endpoints (this method), never in
# bulk over every guild the bot is in — see _guilds_list.
try:
member = await guild.fetch_member(user_id)
except (discord.NotFound, discord.HTTPException):
return False
return member.guild_permissions.manage_guild or member.guild_permissions.administrator