Skip to content

Security/input tag sanitization and choose multiple dedup#58

Merged
choonkeat merged 2 commits intomainfrom
security/input-tag-sanitization-and-choose-multiple-dedup
Apr 16, 2026
Merged

Security/input tag sanitization and choose multiple dedup#58
choonkeat merged 2 commits intomainfrom
security/input-tag-sanitization-and-choose-multiple-dedup

Conversation

@choonkeat
Copy link
Copy Markdown
Owner

Summary

Two independent security hardenings found during a manual review of the form-definition decode path and the Go server-side validator.

1. Custom-element tag + attribute sanitization (XSS hardening)

A form definition loaded from an untrusted source (e.g. the demo's URL hash in index.html) could specify inputTag: "iframe" with attributes: { "srcdoc": "<script>…</script>" }. Elm's VirtualDom only rewrites on* / formAction attribute keys and only blanks values starting with javascript: or data:text/html,, so srcdoc slipped through both filters. The iframe runs its inline HTML in the embedder's origin, yielding same-origin script execution (cookies, CSRF, DOM takeover).

Fix (src/Main.elm): sanitize at decode time.

  • sanitizeInputTag — accept only "input" (default) or a valid hyphenated custom-element name (per the HTML custom-elements spec). Anything else (iframe, object, embed, meta, script, <foo!>, etc.) silently falls back to "input".
  • sanitizeAttributes — drop attribute keys srcdoc, sandbox, allow, csp, http-equiv, background (script-execution or iframe-boundary loosening). Case-insensitive.
  • Applied in both decodeCustomElement and decodeInputTagAttributes.

Impact of the change on existing usage: no known legitimate use in this repo or the demo is affected. Native input and hyphenated custom elements like validated-input pass through unchanged. style is not in the denylist — inline styling continues to work.

Tests: tests/XssDefenseTest.elm — 19 cases covering tag allowlist (input, validated-input, iframe, IFrame, object, embed, meta, script, -foo, my!tag), attribute denylist (srcdoc, sandbox, allow, http-equiv, case-insensitive), benign pass-through (type/pattern/maxlength/placeholder/style), and end-to-end neutralization of the exact iframe+srcdoc PoC.

2. ChooseMultiple dedup in Go validator

ChooseMultipleField.Validate counted len(value) directly against MinRequired and MaxAllowed. A raw POST body can repeat the same name=value pair (colors=Red&colors=Red&colors=Red), which satisfied a "pick at least 3" rule with one real selection. Browsers don't submit checkboxes that way and the Elm frontend dedupes, so this only surfaces on hand-crafted HTTP requests — but the Go validator is the server-side backstop and shouldn't be fooled by it.

Fix (go/validate.go): new uniqueStrings helper; dedupe before evaluating MinRequired, MaxAllowed, and the per-value choice check. Behavior on well-formed (non-duplicate) submissions is unchanged.

Tests (go/validate_test.go):

  • TestChooseMultipleDedupsMinRequired — 3 distinct passes; 3 copies of one fails minRequired: 3; 2 distinct + 1 duplicate also fails.
  • TestChooseMultipleDedupsMaxAllowed — 2 distinct passes; 3 copies of one passes (one distinct); 3 distinct exceeds maxAllowed: 2.

A form definition loaded from an untrusted source (e.g. the demo's
URL hash) could specify inputTag "iframe" with attributes.srcdoc to
render <iframe srcdoc="<script>...</script>">. Elm's VirtualDom only
rewrites on*/formAction keys and blanks javascript:/data:text/html,
value prefixes, so srcdoc slipped through — the iframe runs inline
HTML in the embedder's origin.

Sanitize at decode time:
- inputTag must be "input" or a valid hyphenated custom-element name;
  anything else falls back to "input" (blocks iframe, object, embed,
  meta, script, etc.).
- Drop attributes whose keys are srcdoc, sandbox, allow, csp,
  http-equiv, or background (defense in depth against the remaining
  script-execution / boundary-loosening attributes).

Applied in decodeCustomElement and decodeInputTagAttributes. Covered
by tests/XssDefenseTest.elm (19 cases).
ChooseMultipleField.Validate counted len(value) directly against
MinRequired and MaxAllowed. A raw POST body can repeat the same
name=value pair ("colors=Red&colors=Red&colors=Red"), which satisfied
"pick at least N" with one real selection. Browsers don't submit
checkboxes that way and the Elm frontend dedupes, so this only
surfaces on hand-crafted HTTP submissions, but the Go validator is
the server-side backstop and shouldn't be fooled by it.

Dedupe via a new uniqueStrings helper before evaluating MinRequired,
MaxAllowed, and the per-value choice check. Behavior for well-formed
(non-duplicate) submissions is unchanged. New tests cover duplicate
submissions failing minRequired, passing maxAllowed when collapsed,
and the normal happy paths.
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 14, 2026

Deploy Preview for tiny-form-fields ready!

Name Link
🔨 Latest commit a46517c
🔍 Latest deploy log https://app.netlify.com/projects/tiny-form-fields/deploys/69de1dc1f47f460008a54218
😎 Deploy Preview https://deploy-preview-58--tiny-form-fields.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread go/validate.go
Comment on lines +341 to +345
// Count distinct submissions. A raw POST body can repeat the same value
// (e.g. "color=Red&color=Red&color=Red"), which would otherwise let a
// caller satisfy MinRequired with a single distinct choice or inflate
// the count past MaxAllowed inspection.
distinct := uniqueStrings(value)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 shouldnt break legitimate users on goventry since a real browser form submission wouldnt have dup checkbox values

Comment thread src/Main.elm
Comment on lines +4069 to +4076
-- XSS defense. Elm's VirtualDom rewrites on*/formAction keys and blanks
-- javascript:/data:text/html, values, but leaves the tag name and other
-- attributes untouched. A form definition can reach both, so we normalize
-- here: reject any inputTag that isn't the default "input" or a valid
-- custom-element name (must contain a hyphen per the HTML spec; that alone
-- rules out iframe/object/embed/meta/etc.), and drop attributes whose
-- name or value can still produce script execution or boundary-loosening
-- on the native tags we do allow.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt break goventry too since all our input tags and attributes matches the allowed list

@choonkeat choonkeat merged commit 37e150f into main Apr 16, 2026
6 checks passed
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.

2 participants