Fix XSS exploit in metadata_url (#1107)

* Remove trellis. * Failing test. * Return nil unless santitised string matches original. * Format and translations. * Yolo upload-artifact to v4 for github ci. * Fix another outdated upload action.
celo-org · Oct 30, 2024 · 004cf3f · Covitluat77 · Dec 20, 2024 · 004cf3f
1 parent 0d33ce2
commit 004cf3f
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 13 deletions.
diff --git a/.github/workflows/blockscout.yml b/.github/workflows/blockscout.yml
@@ -300,7 +300,7 @@ jobs:
 
       - name: Upload Unit Test Results
         if: always()
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
         with:
           name: ESLint Test Results
           path: apps/block_scout_web/assets/test/eslint/*.xml
@@ -348,7 +348,7 @@ jobs:
 
       - name: Upload Unit Test Results
         if: always()
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
         with:
           name: Jest JUnit Test Results
           path: apps/block_scout_web/assets/junit.xml
@@ -417,7 +417,7 @@ jobs:
           ETHEREUM_JSONRPC_WEB_SOCKET_CASE: "EthereumJSONRPC.WebSocket.Case.Mox"
       - name: Upload Unit Test Results
         if: always()
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
         with:
           name: EthereumJSONRPC Test Results
           path: _build/test/junit/ethereum_jsonrpc/*.xml
@@ -506,7 +506,7 @@ jobs:
           ETHEREUM_JSONRPC_WEB_SOCKET_CASE: "EthereumJSONRPC.WebSocket.Case.Mox"
       - name: Upload Unit Test Results
         if: always()
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
         with:
           name: Explorer Test Results
           path: _build/test/junit/explorer/*.xml
@@ -578,7 +578,7 @@ jobs:
           ETHEREUM_JSONRPC_WEB_SOCKET_CASE: "EthereumJSONRPC.WebSocket.Case.Mox"
       - name: Upload Unit Test Results
         if: always()
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
         with:
           name: Indexer Test Results
           path: _build/test/junit/indexer/*.xml
@@ -687,14 +687,14 @@ jobs:
           API_V2_ENABLED: "true"
       - name: Upload Unit Test Results
         if: always()
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
         with:
           name: Blockscout Web Test Results
           path: _build/test/junit/block_scout_web/*.xml
 
       - name: Upload Wallaby screenshots
         if: always()
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
         with:
           name: Wallaby screenshots
           path: apps/block_scout_web/screenshots/*.png
@@ -718,7 +718,7 @@ jobs:
           path: artifacts
 
       - name: Publish Unit Test Results
-        uses: EnricoMi/publish-unit-test-result-action@v1
+        uses: EnricoMi/publish-unit-test-result-action@v2
         with:
           files: artifacts/**/*.xml
 

diff --git a/apps/block_scout_web/lib/block_scout_web/views/tokens/instance/overview_view.ex b/apps/block_scout_web/lib/block_scout_web/views/tokens/instance/overview_view.ex
@@ -102,7 +102,15 @@ defmodule BlockScoutWeb.Tokens.Instance.OverviewView do
 
   def external_url(nil), do: nil
 
-  def external_url("http" <> _rest = external_url), do: external_url
+  def external_url("http" <> _rest = external_url) do
+    sanitised = external_url |> html_escape() |> safe_to_string()
+
+    if sanitised != external_url do
+      nil
+    else
+      external_url
+    end
+  end
 
   def external_url(string) when is_binary(string), do: external_url(nil)
 

diff --git a/apps/block_scout_web/priv/gettext/default.pot b/apps/block_scout_web/priv/gettext/default.pot
@@ -1642,7 +1642,7 @@ msgstr ""
 
 #: lib/block_scout_web/templates/tokens/instance/metadata/index.html.eex:18
 #: lib/block_scout_web/templates/tokens/instance/overview/_tabs.html.eex:10
-#: lib/block_scout_web/views/tokens/instance/overview_view.ex:202
+#: lib/block_scout_web/views/tokens/instance/overview_view.ex:210
 #, elixir-autogen, elixir-format
 msgid "Metadata"
 msgstr ""
@@ -2659,7 +2659,7 @@ msgstr ""
 #: lib/block_scout_web/templates/transaction/_tabs.html.eex:4
 #: lib/block_scout_web/templates/transaction_token_transfer/index.html.eex:7
 #: lib/block_scout_web/views/address_view.ex:434
-#: lib/block_scout_web/views/tokens/instance/overview_view.ex:201
+#: lib/block_scout_web/views/tokens/instance/overview_view.ex:209
 #: lib/block_scout_web/views/tokens/overview_view.ex:39
 #: lib/block_scout_web/views/transaction_view.ex:535
 #, elixir-autogen, elixir-format

diff --git a/apps/block_scout_web/priv/gettext/en/LC_MESSAGES/default.po b/apps/block_scout_web/priv/gettext/en/LC_MESSAGES/default.po
@@ -1642,7 +1642,7 @@ msgstr ""
 
 #: lib/block_scout_web/templates/tokens/instance/metadata/index.html.eex:18
 #: lib/block_scout_web/templates/tokens/instance/overview/_tabs.html.eex:10
-#: lib/block_scout_web/views/tokens/instance/overview_view.ex:202
+#: lib/block_scout_web/views/tokens/instance/overview_view.ex:210
 #, elixir-autogen, elixir-format
 msgid "Metadata"
 msgstr ""
@@ -2659,7 +2659,7 @@ msgstr ""
 #: lib/block_scout_web/templates/transaction/_tabs.html.eex:4
 #: lib/block_scout_web/templates/transaction_token_transfer/index.html.eex:7
 #: lib/block_scout_web/views/address_view.ex:434
-#: lib/block_scout_web/views/tokens/instance/overview_view.ex:201
+#: lib/block_scout_web/views/tokens/instance/overview_view.ex:209
 #: lib/block_scout_web/views/tokens/overview_view.ex:39
 #: lib/block_scout_web/views/transaction_view.ex:535
 #, elixir-autogen, elixir-format

diff --git a/apps/block_scout_web/test/block_scout_web/views/tokens/instance/overview_view_test.exs b/apps/block_scout_web/test/block_scout_web/views/tokens/instance/overview_view_test.exs
@@ -138,6 +138,16 @@ defmodule BlockScoutWeb.Tokens.Instance.OverviewViewTest do
       assert result == nil, "non http url schemes should be stripped from external_url and treated as missing"
     end
 
+    test "does not return html escape" do
+      data = %{
+        "external_url" => "https\" id=x tabindex=1 onfocusin=eval(atob('KGZ1bmN0aW9uKCl7d2luZG93LmV0aG'))"
+      }
+
+      result = OverviewView.external_url(%{metadata: data})
+
+      assert result == nil, "non http url schemes should be stripped from external_url and treated as missing"
+    end
+
     test "Returns valid uri scheme" do
       json = """
         {
@@ -153,6 +163,7 @@ defmodule BlockScoutWeb.Tokens.Instance.OverviewViewTest do
       result = OverviewView.external_url(%{metadata: data})
 
       assert String.starts_with?(result, "http"), "Valid url should be returned"
+      assert result == "https://happyland.nft"
     end
   end
 end