Skip to content

Escape PR html_url in generated report to prevent HTML injection - #1

Open
kwsantiago wants to merge 1 commit into
luke-jr:knots-specfrom
privkeyio:escape-html-url-in-report
Open

Escape PR html_url in generated report to prevent HTML injection#1
kwsantiago wants to merge 1 commit into
luke-jr:knots-specfrom
privkeyio:escape-html-url-in-report

Conversation

@kwsantiago

Copy link
Copy Markdown

In geninfo-to-html.pl, the PR title is escaped via preptitle -> encode_entities, but the adjacent html_url is interpolated straight into the href attribute with no escaping:

$_ .= " href=\"" . $j->{"html_url"} . "\">" . $subject . "</a>";

$j is decode_json of the GitHub API response (also persisted to the local cache), so the href value is network/cache-sourced. If that value ever contains a "/</> (a malformed/compromised API response, a tampered cache file, or a javascript: URI), it breaks out of the attribute and injects arbitrary markup/JS into the report when it is opened in a browser.

This escapes the URL the same way the title already is, and blocks non-http(s) schemes:

my $url = $j->{"html_url"} // '';
$url = '' unless $url =~ m{^https?://}i;
$_ .= " href=\"" . encode_entities($url, '<>&"') . "\">" . $subject . "</a>";

Verified: output is byte-identical for legitimate GitHub URLs (no-op), and a malicious html_url that previously injected <script> now renders as an empty href. The unrelated BM/LA commit-link path is unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant