naruto-lgtm opened a new pull request, #1455: URL: https://github.com/apache/poi/pull/1455
Repro: change the target of the HYPERLINK field in hyperlink.doc from `http://testuri.org/` to `javascript:alert(1)` (the new test does this in the WordDocument stream) and convert it with `WordToHtmlConverter`. The output contains `<a href="javascript:alert(1)">`, so a .doc converted to HTML for display runs script in the origin serving the page once the link is clicked (stored XSS). The serializer escapes the attribute value, so the problem is the scheme, not markup breakout. Cause: `WordToHtmlConverter.processHyperlink` hands the target parsed from the field code to `createHyperlink` as it is. Fix: `WordToHtmlUtils.isSafeHyperlink` reads the scheme the way browsers do (tabs and line breaks dropped, leading control characters and spaces ignored) and accepts targets without a scheme (relative paths, fragments, UNC paths), drive paths like `C:\`, and http, https, ftp, mailto, file, news and tel. Any other target keeps its text but is not emitted as a link. The check sits in the converter rather than in `HtmlDocumentFacade.createHyperlink`, which also builds the internal `#bookmark` links. The trade-off of an allowlist is that a link with an uncommon scheme loses its href in the HTML output; extending the set is a one-line change. Output for hyperlink.doc and the mailto link in Bug47286.doc is unchanged, and `:poi-scratchpad:check` passes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
