fix: Implement URL sanitization for DEX and website links #3
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added a sanitizeHref function to validate and sanitize URLs before use.
Extracting text from a DOM node and interpreting it as HTML can lead to a cross-site scripting vulnerability. A webpage with this vulnerability reads text from the DOM, and afterwards adds the text as HTML to the DOM. Using text from the DOM as HTML effectively unescapes the text, and thereby invalidates any escaping done on the text. If an attacker is able to control the safe sanitized text, then this vulnerability can be exploited to perform a cross-site scripting attack.
In general, untrusted strings used in URL contexts should be validated and normalized before being placed in
hrefor similar attributes. Specifically, you should only allow safe URL schemes (http:,https:– optionally others likemailto:if needed) and reject or neutralize dangerous schemes likejavascript:,data:,vbscript:, etc. This can be done either where input is accepted (sanitizingwebsiteUrlbefore storing it) or at the point of use (sanitizingbroker.dexUrlbefore rendering it).The least intrusive and most robust fix here is to introduce a small URL-cleaning helper in
DexCard.tsxand use it aroundbroker.dexUrl(and, for consistency/safety, aroundbroker.websiteUrltoo) before they are passed tohref. The helper should:string | null | undefined."#"(the current safe fallback).new URL()either with the string as-is (absolute) or relative towindow.location.originwhen in the browser (to handle inputs like"/path"). If parsing fails, fall back to"#".url.protocolis in an allowed list (e.g.,http:andhttps:). If not, fall back to"#".This keeps existing behavior for valid HTTP(S) URLs and relative paths, while preventing
javascript:or other malicious schemes from being used. Implementation-wise:app/app/components/DexCard.tsx, definesanitizeHrefabove the component.href={broker.dexUrl || "#"}withhref={sanitizeHref(broker.dexUrl)}.href={broker.websiteUrl}withhref={sanitizeHref(broker.websiteUrl)}.