Skip to content

Fix: Stored XSS via article.link_to in _ArticleController - #195

Open
milo2012 wants to merge 1 commit into
JPressProjects:v5.xfrom
milo2012:fix/security-vulnerability
Open

Fix: Stored XSS via article.link_to in _ArticleController#195
milo2012 wants to merge 1 commit into
JPressProjects:v5.xfrom
milo2012:fix/security-vulnerability

Conversation

@milo2012

Copy link
Copy Markdown

Pull Request: Fix stored XSS via article.link_to in _ArticleController (/admin/article/doWriteSave)

Summary

Fixes a stored cross-site scripting (XSS) vulnerability in the article
writing endpoint /admin/article/doWriteSave. The article.link_to
field (external link) was persisted without URL validation and later
rendered unescaped in HTML attribute/href contexts, allowing a malicious
payload to execute JavaScript in an administrator's or visitor's browser.

Vulnerable file

  • module-article/module-article-web/src/main/java/io/jpress/module/article/controller/admin/_ArticleController.java

Root cause

  1. doWriteSave() binds the raw request into Article via getModel(...).
    JFinal's model injection does not escape parameter values.

  2. Article.save()/update() pass link_to through
    CommonsUtils.escapeModel + JsoupUtils.clean, but Jsoup cleaning only
    strips HTML tags. It does NOT stop attribute-breakout payloads because
    a double quote is a valid quote to the HTML parser inside an attribute,
    e.g.:
    " onmouseover="alert(document.cookie)" data-x="

  3. The stored value is later output with NO HTML escaping:

    • admin write page: value="#unescape(article.link_to ??)" (article_write.html)
    • admin article list: <a href="#(item.url ??)">查看</a> (article_list.html)
    • front-end templates:<a href="#(article.url ??)"> (== Article.getUrl() == getLinkTo())

    Result: hovering a crafted link fires JavaScript.

Fix

Added input validation validateLinkTo(Article) in _ArticleController,
called at the top of doWriteSave() (before validateSlug). When the
submitted link_to value is unsafe, the request is rejected with
Ret.fail("message", ...) and nothing is persisted.

The validation:

  • strips invisible control characters (blocks \n/\t-based protocol- or
    character-bypass attempts),
  • rejects values containing <, >, ", ', ` (blocks HTML tag and
    attribute injection),
  • rejects dangerous URL schemes: javascript:, vbscript:, data:, file:
    (allows http/https-jsoup-style links, relative paths and anchors).

Files changed

  • module-article/module-article-web/src/main/java/io/jpress/module/article/controller/admin/_ArticleController.java

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