Reject javascript: and vbscript: URI schemes in href (#4113)

Author: liuliu-dev

.changeset/sanitize-href-uri-scheme.md

@@ -0,0 +1,5 @@
+---
+"@primer/view-components": patch
+---
+
+Reject `javascript:` and `vbscript:` URI schemes in `href` (defense in depth). When a component (e.g. `Primer::Beta::Label`, `Primer::Beta::Button`, `Primer::Beta::Link`) is rendered as an anchor with an unsafe `href`, the value is now rejected — raising in non-production environments and silently dropped (rendered as an anchor with no `href`) in production.

app/components/primer/base_component.rb

@@ -156,6 +156,7 @@ def initialize(tag:, classes: nil, **system_arguments)
       @tag = tag
 
       @system_arguments = validate_arguments(tag: tag, **system_arguments)
+      sanitize_href!(@system_arguments)
 
       @result = Primer::Classify.call(**@system_arguments.merge(classes: classes))
 

app/components/primer/component.rb

@@ -22,6 +22,7 @@ class Component < ViewComponent::Base
     include AttributesHelper
     include ClassNameHelper
     include FetchOrFallbackHelper
+    include SafeHrefHelper
     include TestSelectorHelper
     include JoinStyleArgumentsHelper
     include ViewHelper
@@ -137,6 +138,21 @@ def deny_tag_argument(**arguments)
       deny_single_argument(:tag, "This component has a fixed tag.", **arguments)
     end
 
+    # Removes `href` values that point at disallowed URI schemes (`javascript:`,
+    # `vbscript:`). Raises in non-production environments so the offending call
+    # site gets fixed; in production we silently drop the attribute so the link
+    # is rendered inert rather than crashing the page.
+    def sanitize_href!(arguments)
+      return unless arguments.key?(:href)
+      return unless Primer::SafeHrefHelper.unsafe_href?(arguments[:href])
+
+      if should_raise_error?
+        raise ArgumentError, "Rejected dangerous URI scheme in `href`: #{arguments[:href].inspect}"
+      end
+
+      arguments[:href] = nil
+    end
+
     def should_raise_error?
       !Rails.env.production? && raise_on_invalid_options? && !ENV["PRIMER_WARNINGS_DISABLED"]
     end

app/lib/primer/safe_href_helper.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+# Primer::SafeHrefHelper
+#
+# Detects unsafe URI schemes (e.g. `javascript:`) in `href` values so they can
+# be neutralized before being rendered into HTML attributes. Without this check,
+# a caller that forwards untrusted input into a component's `href:` argument can
+# trigger XSS when the user clicks the rendered anchor.
+module Primer
+  # :nodoc:
+  module SafeHrefHelper
+    # URI schemes that can execute script in the browser and are never valid
+    # destinations for a Primer-rendered link.
+    DISALLOWED_HREF_SCHEMES = %w[javascript vbscript].freeze
+
+    # Returns true when `href` starts with a disallowed URI scheme.
+    #
+    # Mirrors browser URL parsing by stripping ASCII whitespace and control
+    # characters (including tab/CR/LF) before extracting the scheme. This
+    # prevents bypasses such as `j\tavascript:...`, ` JaVaScRiPt:...`, or a
+    # leading null byte, all of which browsers happily execute.
+    def self.unsafe_href?(href)
+      return false if href.nil?
+
+      normalized = href.to_s.gsub(/[\u0000-\u0020]/, "")
+      scheme = normalized[/\A([a-z][a-z0-9+\-.]*):/i, 1]
+      return false unless scheme
+
+      DISALLOWED_HREF_SCHEMES.include?(scheme.downcase)
+    end
+
+    def unsafe_href?(href)
+      SafeHrefHelper.unsafe_href?(href)
+    end
+  end
+end

test/components/base_component_test.rb

@@ -92,6 +92,46 @@ def test_renders_as_a_link
     assert_selector("a[href='http://google.com']")
   end
 
+  def test_rejects_javascript_href_in_non_production
+    err = assert_raises(ArgumentError) do
+      render_inline(Primer::BaseComponent.new(tag: :a, href: "javascript:alert(1)"))
+    end
+    assert_match(/Rejected dangerous URI scheme/, err.message)
+  end
+
+  def test_neutralizes_javascript_href_in_production
+    with_raise_on_invalid_options(false) do
+      render_inline(Primer::BaseComponent.new(tag: :a, href: "javascript:alert(1)")) { "x" }
+
+      assert_selector("a")
+      refute_selector("a[href]")
+    end
+  end
+
+  def test_neutralizes_vbscript_href_in_production
+    with_raise_on_invalid_options(false) do
+      render_inline(Primer::BaseComponent.new(tag: :a, href: "vbscript:msgbox(1)")) { "x" }
+
+      refute_selector("a[href]")
+    end
+  end
+
+  def test_neutralizes_javascript_href_with_whitespace_bypass_attempt
+    with_raise_on_invalid_options(false) do
+      render_inline(Primer::BaseComponent.new(tag: :a, href: "\tJaVaScRiPt:alert(1)")) { "x" }
+
+      refute_selector("a[href]")
+    end
+  end
+
+  def test_allows_safe_hrefs
+    render_inline(Primer::BaseComponent.new(tag: :a, href: "/foo/bar"))
+    assert_selector("a[href='/foo/bar']")
+
+    render_inline(Primer::BaseComponent.new(tag: :a, href: "mailto:hello@example.com"))
+    assert_selector("a[href='mailto:hello@example.com']")
+  end
+
   # We were calling tag.send(as), passing in :p ended up calling `p`, aka `puts`
   # Due to how Rails uses method_missing in TagHelper. See Slack convo:
   # https://github.slack.com/archives/C0HV3F37A/p1556216733019500

test/components/beta/label_test.rb

@@ -95,4 +95,19 @@ def test_falls_back_when_variant_isn_t_valid
   def test_status
     assert_component_state(Primer::Beta::Label, :beta)
   end
+
+  def test_rejects_javascript_href_when_tag_is_anchor
+    assert_raises(ArgumentError) do
+      render_inline(Primer::Beta::Label.new(tag: :a, href: "javascript:alert(document.domain)")) { "x" }
+    end
+  end
+
+  def test_neutralizes_javascript_href_when_tag_is_anchor_in_production
+    with_raise_on_invalid_options(false) do
+      render_inline(Primer::Beta::Label.new(tag: :a, href: "javascript:alert(document.domain)")) { "x" }
+
+      assert_selector("a.Label")
+      refute_selector("a.Label[href]")
+    end
+  end
 end

test/components/primer/beta/button_test.rb

@@ -87,4 +87,19 @@ def test_warns_on_uses_of_dropdown
 
     assert_equal "The `dropdown:` argument is no longer supported on Primer::Beta::Button. Use the `trailing_action` slot instead.", err.message
   end
+
+  def test_rejects_javascript_href_when_rendered_as_anchor
+    assert_raises(ArgumentError) do
+      render_inline(Primer::Beta::Button.new(tag: :a, href: "javascript:alert(document.domain)")) { "Button" }
+    end
+  end
+
+  def test_neutralizes_javascript_href_when_rendered_as_anchor_in_production
+    with_raise_on_invalid_options(false) do
+      render_inline(Primer::Beta::Button.new(tag: :a, href: "javascript:alert(document.domain)")) { "Button" }
+
+      assert_selector("a.Button", text: "Button")
+      refute_selector("a.Button[href]")
+    end
+  end
 end

test/lib/safe_href_helper_test.rb

@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+require "lib/test_helper"
+
+class Primer::SafeHrefHelperTest < Minitest::Test
+  include Primer::SafeHrefHelper
+
+  def test_returns_false_for_nil
+    refute Primer::SafeHrefHelper.unsafe_href?(nil)
+  end
+
+  def test_returns_false_for_relative_paths
+    refute Primer::SafeHrefHelper.unsafe_href?("/foo/bar")
+    refute Primer::SafeHrefHelper.unsafe_href?("foo/bar")
+    refute Primer::SafeHrefHelper.unsafe_href?("#anchor")
+    refute Primer::SafeHrefHelper.unsafe_href?("?query=1")
+    refute Primer::SafeHrefHelper.unsafe_href?("")
+  end
+
+  def test_returns_false_for_safe_schemes
+    refute Primer::SafeHrefHelper.unsafe_href?("https://example.com")
+    refute Primer::SafeHrefHelper.unsafe_href?("http://example.com")
+    refute Primer::SafeHrefHelper.unsafe_href?("mailto:foo@example.com")
+    refute Primer::SafeHrefHelper.unsafe_href?("ftp://example.com")
+    refute Primer::SafeHrefHelper.unsafe_href?("tel:+15551234567")
+    refute Primer::SafeHrefHelper.unsafe_href?("data:image/png;base64,abc")
+  end
+
+  def test_detects_javascript_uri
+    assert Primer::SafeHrefHelper.unsafe_href?("javascript:alert(1)")
+  end
+
+  def test_detects_vbscript_uri
+    assert Primer::SafeHrefHelper.unsafe_href?("vbscript:msgbox(1)")
+  end
+
+  def test_detects_mixed_case_schemes
+    assert Primer::SafeHrefHelper.unsafe_href?("JaVaScRiPt:alert(1)")
+    assert Primer::SafeHrefHelper.unsafe_href?("JAVASCRIPT:alert(1)")
+    assert Primer::SafeHrefHelper.unsafe_href?("VbScript:alert(1)")
+  end
+
+  def test_detects_leading_whitespace_bypasses
+    assert Primer::SafeHrefHelper.unsafe_href?(" javascript:alert(1)")
+    assert Primer::SafeHrefHelper.unsafe_href?("\tjavascript:alert(1)")
+    assert Primer::SafeHrefHelper.unsafe_href?("\njavascript:alert(1)")
+    assert Primer::SafeHrefHelper.unsafe_href?("\rjavascript:alert(1)")
+  end
+
+  def test_detects_embedded_whitespace_and_control_chars_in_scheme
+    # Browsers strip tab/CR/LF inside URLs per the WHATWG URL spec, so these execute.
+    assert Primer::SafeHrefHelper.unsafe_href?("java\tscript:alert(1)")
+    assert Primer::SafeHrefHelper.unsafe_href?("java\nscript:alert(1)")
+    assert Primer::SafeHrefHelper.unsafe_href?("java\rscript:alert(1)")
+    assert Primer::SafeHrefHelper.unsafe_href?("j\u0000avascript:alert(1)")
+  end
+
+  def test_does_not_misclassify_schemes_that_merely_contain_javascript
+    refute Primer::SafeHrefHelper.unsafe_href?("https://example.com/javascript:foo")
+    refute Primer::SafeHrefHelper.unsafe_href?("/javascript:foo")
+  end
+
+  def test_handles_non_string_values
+    refute Primer::SafeHrefHelper.unsafe_href?(123)
+    refute Primer::SafeHrefHelper.unsafe_href?(:foo)
+  end
+
+  def test_instance_method_delegates_to_module_method
+    assert unsafe_href?("javascript:alert(1)")
+    refute unsafe_href?("https://example.com")
+  end
+end