elm / virtual-dom

The foundation of HTML and SVG in Elm.
https://package.elm-lang.org/packages/elm/virtual-dom/latest
BSD 3-Clause "New" or "Revised" License
209 stars 80 forks source link

Possible to bypass XSS filter with nodeNS #168

Closed hpate-omicron closed 2 years ago

hpate-omicron commented 4 years ago

SSCCE: Ellie

module Main exposing (main)

import Html
import VirtualDom exposing (nodeNS)

main =
    nodeNS "http://www.w3.org/2000/svg"
        "script"
        []
        [ Html.text "alert('Hi')" ]

The user John J on Slack found it, he had this research. Slack link

The crux of it seems to be that nodeNS arguments are swapped between Elm and JS, on the elm side the tag is the first argument passed into nodeNS. Link to code

nodeNS : String -> String -> List (Attribute msg) -> List (Node msg) -> Node msg
nodeNS tag =
  Elm.Kernel.VirtualDom.nodeNS (Elm.Kernel.VirtualDom.noScript tag)

But on the JS side it is expecting the tag as the second argument

var _VirtualDom_nodeNS = F2(function(namespace, tag)
lydell commented 3 years ago

The crux of it seems to be that nodeNS arguments are swapped between Elm and JS, on the elm side the tag is the first argument passed into nodeNS

This is close, but not quite what the issue is.

Compare nodeNS:

nodeNS : String -> String -> List (Attribute msg) -> List (Node msg) -> Node msg
nodeNS tag =
  Elm.Kernel.VirtualDom.nodeNS (Elm.Kernel.VirtualDom.noScript tag)

…with keyedNodeNS:

keyedNodeNS : String -> String -> List (Attribute msg) -> List ( String, Node msg ) -> Node msg
keyedNodeNS namespace tag =
  Elm.Kernel.VirtualDom.keyedNodeNS namespace (Elm.Kernel.VirtualDom.noScript tag)

nodeNS first parameter is named wrong, and noScript is uselessly called on it. Here’s my suggestion:

nodeNS : String -> String -> List (Attribute msg) -> List (Node msg) -> Node msg
nodeNS namespace tag =
  Elm.Kernel.VirtualDom.nodeNS namespace (Elm.Kernel.VirtualDom.noScript tag)
wclr commented 3 years ago

Seems there is just just a little mistake made by @evancz.

evancz commented 2 years ago

Fixed in https://github.com/elm/virtual-dom/commit/9a389f84978930ca2f1dc2f84db505dafa018d3c which should be published shortly with version 1.0.3. Thank you for the report, and thank you to @jfmengels for organizing the XSS issues and coordinating on testing.