Skip to content

Strip control chars from URLs in jsoup StringUtil - #323

Open
Chandan3456 wants to merge 1 commit into
itext:developfrom
Chandan3456:jsoup-url-control-chars
Open

Chandan3456 wants to merge 1 commit into
itext:developfrom
Chandan3456:jsoup-url-control-chars

Conversation

@Chandan3456

Copy link
Copy Markdown

the jsoup port's Cleaner checks allowed protocols against the url resolved by StringUtil.resolve, but java.net.URL keeps ascii control characters that browsers strip, so the check and the browser disagree on the scheme. with Safelist.basic().preserveRelativeLinks(true), resolves relative to the base url, passes the protocol check, and the raw value survives cleaning; a browser then strips the tab and runs javascript:alert(1). same for CR/LF and / entity variants. this is CVE-2022-36033, fixed upstream in jsoup 1.15.3 (jhy/jsoup@4ea768d); this ports that fix to the vendored copy by stripping ascii 0x00-0x1f from urls in StringUtil.resolve before resolution. the ported regression tests fail before the change and pass after, and the styled-xml-parser and svg unit suites stay green.

The jsoup Cleaner validates link protocols against the resolved
absolute URL, but java.net.URL keeps ASCII control characters that
browsers strip, so the check and the browser can disagree about the
scheme. With Safelist.preserveRelativeLinks(true) an href such as
java&itext#9;script:alert(1) passes the protocol check and is kept as is
in the sanitized output, where a browser strips the tab and runs it
as javascript: (CVE-2022-36033 in upstream jsoup). Port the jsoup
1.15.3 fix (jhy/jsoup@4ea768d): remove ASCII control characters
from URLs in StringUtil.resolve before resolution.
@iText-CI

Copy link
Copy Markdown
Contributor

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@vitali-pr

Copy link
Copy Markdown
Contributor

@Chandan3456 , indeed, we have in near plans to update our port to the latest jsoup version where we will address all known vulnerabilities in one go.

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.

3 participants