Security fixes: sanitize exception messages, CSP URL check, bump yt-dlp#273
Security fixes: sanitize exception messages, CSP URL check, bump yt-dlp#273
Conversation
…SP URL check, bump yt-dlp
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| maps_endpoint = app.config.get("MAPS_API_ENDPOINT", "") | ||
| if "openstreetmap" in maps_endpoint or "tile.osm.org" in maps_endpoint: | ||
| _maps_host = urlparse(maps_endpoint).hostname or "" | ||
| if _maps_host.endswith("openstreetmap.org") or _maps_host.endswith("tile.osm.org"): |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
In general, the problem is that the code uses endswith("openstreetmap.org") and endswith("tile.osm.org") directly on the hostname, which allows hosts like evil-openstreetmap.org or malicious-tile.osm.org to be treated as if they were legitimate OSM domains. The fix is to perform a proper host validation: either require an exact match of the known hostnames or allow only true subdomains by checking for a preceding dot. This way, tile.openstreetmap.org and sub.tile.openstreetmap.org are accepted, but evil-openstreetmap.org is not.
The best way to fix this without changing the existing functionality is:
- Keep the use of
urlparseto extract the hostname. - Replace the direct
endswithchecks with a helper that enforces:- Either exactly the allowed domain (e.g.
tile.openstreetmap.org), or - A subdomain that ends with
.+ allowed domain (e.g.a.tile.openstreetmap.org).
- Either exactly the allowed domain (e.g.
- Use this helper for both
openstreetmap.organdtile.osm.org.
To implement this in enferno/app.py:
- Define a small local helper function inside
register_talisman(so you don’t change the module-level API) that takes ahostnameand adomainand returnsTrueif the hostname matches the domain exactly or is a subdomain of it. - Replace the condition on line 157 with one that calls this helper for
"openstreetmap.org"and"tile.osm.org".
No new imports are needed; we can implement the helper using basic string operations.
Concretely:
- In
register_talisman, just before computing_maps_host, add the helper functionis_allowed_domain(hostname, domain). - Replace the existing
if _maps_host.endswith("openstreetmap.org") or _maps_host.endswith("tile.osm.org"):with a call tois_allowed_domain(_maps_host, "openstreetmap.org")andis_allowed_domain(_maps_host, "tile.osm.org").
This preserves behavior for legitimate OSM hosts while preventing accidental matches for arbitrary domains that just end in those strings.
| @@ -151,10 +151,21 @@ | ||
| "base-uri": "'self'", | ||
| } | ||
|
|
||
| def is_allowed_domain(hostname: str, domain: str) -> bool: | ||
| """ | ||
| Return True if the hostname matches the given domain exactly | ||
| or is a subdomain of it. | ||
| """ | ||
| if not hostname: | ||
| return False | ||
| hostname = hostname.lower() | ||
| domain = domain.lower() | ||
| return hostname == domain or hostname.endswith("." + domain) | ||
|
|
||
| # Add map tile servers to img-src and connect-src | ||
| maps_endpoint = app.config.get("MAPS_API_ENDPOINT", "") | ||
| _maps_host = urlparse(maps_endpoint).hostname or "" | ||
| if _maps_host.endswith("openstreetmap.org") or _maps_host.endswith("tile.osm.org"): | ||
| if is_allowed_domain(_maps_host, "openstreetmap.org") or is_allowed_domain(_maps_host, "tile.osm.org"): | ||
| csp["img-src"].append("https://tile.osm.org") | ||
| csp["img-src"].append("https://*.tile.osm.org") | ||
| csp["img-src"].append("https://tile.openstreetmap.org") |
| maps_endpoint = app.config.get("MAPS_API_ENDPOINT", "") | ||
| if "openstreetmap" in maps_endpoint or "tile.osm.org" in maps_endpoint: | ||
| _maps_host = urlparse(maps_endpoint).hostname or "" | ||
| if _maps_host.endswith("openstreetmap.org") or _maps_host.endswith("tile.osm.org"): |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
In general, this problem is fixed by ensuring that host checks are based on parsed hostnames and that suffix checks distinguish between the exact domain and its subdomains, instead of matching any string ending in the bare domain name. The recommended pattern is to check host == "example.com" or host.endswith(".example.com") (with a leading dot), depending on whether subdomains should be allowed.
For this specific code in enferno/app.py, we already parse the URL and extract the hostname into _maps_host. We should adjust the conditional on line 157 to correctly recognize tile.osm.org and openstreetmap.org hosts. The safest behavior is likely to allow both the bare domain (e.g., tile.osm.org) and any of its subdomains (e.g., a.tile.osm.org), but not arbitrary suffixes like evil-tile.osm.org.evil.com. To do that without changing other behavior, replace:
if _maps_host.endswith("openstreetmap.org") or _maps_host.endswith("tile.osm.org"):with an explicit check for equality or a dot-suffixed subdomain:
def _is_allowed_osm_host(hostname: str) -> bool:
if not hostname:
return False
allowed_roots = ("tile.osm.org", "tile.openstreetmap.org", "openstreetmap.org")
return any(
hostname == root or hostname.endswith("." + root)
for root in allowed_roots
)and then:
if _is_allowed_osm_host(_maps_host):
...However, we must not change behavior for openstreetmap.org vs tile.openstreetmap.org usage in the CSP targets themselves. The CSP entries we add are all tile subdomains (tile.osm.org, *.tile.osm.org, tile.openstreetmap.org, *.tile.openstreetmap.org). So we only need to ensure we correctly identify when the configured endpoint host is either tile.osm.org, a subdomain of tile.osm.org, or tile.openstreetmap.org / its subdomains. To keep the change minimal, we can inline this logic directly in the if without adding new helpers or imports:
if _maps_host == "tile.osm.org" or _maps_host.endswith(".tile.osm.org") \
or _maps_host == "tile.openstreetmap.org" or _maps_host.endswith(".tile.openstreetmap.org"):
...This avoids over-broad matching (we no longer accept evil-openstreetmap.org) while preserving the intent to recognize official tile servers and their subdomains. No new imports are required, and all changes stay within the shown snippet.
| @@ -154,7 +154,12 @@ | ||
| # Add map tile servers to img-src and connect-src | ||
| maps_endpoint = app.config.get("MAPS_API_ENDPOINT", "") | ||
| _maps_host = urlparse(maps_endpoint).hostname or "" | ||
| if _maps_host.endswith("openstreetmap.org") or _maps_host.endswith("tile.osm.org"): | ||
| if ( | ||
| _maps_host == "tile.osm.org" | ||
| or _maps_host.endswith(".tile.osm.org") | ||
| or _maps_host == "tile.openstreetmap.org" | ||
| or _maps_host.endswith(".tile.openstreetmap.org") | ||
| ): | ||
| csp["img-src"].append("https://tile.osm.org") | ||
| csp["img-src"].append("https://*.tile.osm.org") | ||
| csp["img-src"].append("https://tile.openstreetmap.org") |
Summary
>=2026.2.21to fix high-severity arbitrary command injection via--netrc-cmd(Dependabot alert bynt-1364: implement username and email validation for user endpoints #126)str(e)in error responses with generic messages to prevent internal exception leakage to API clients (CodeQL alert Display hint on username field #132)MAPS_API_ENDPOINTwithurlparsehostname check to prevent crafted URLs from bypassing CSP allowlist (CodeQL alert BYNT-1372: Actor.id_number migration #122)Notes
All three endpoints are behind authentication, but defense-in-depth applies. Exception strings are still logged in full server-side.