Patch external_url method to constrain set of valid uris (#1000)

* Failing test. * Fix test. * Update translations (idk).
celo-org · Dec 7, 2023 · 1b000be · 1b000be
1 parent eec0e90
commit 1b000be
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 6 deletions.
diff --git a/.../block_scout_web/lib/block_scout_web/templates/tokens/instance/overview/_details.html.eex b/.../block_scout_web/lib/block_scout_web/templates/tokens/instance/overview/_details.html.eex
@@ -41,7 +41,7 @@
             <div class="d-flex flex-row justify-content-start text-muted">
               <%= if external_url(@token_instance.instance) do %>
                 <span class="mr-4">
-                  <a data-test="external_url" href=<%=external_url(@token_instance.instance) %> target="_blank">
+                  <a data-test="external_url" href=<%= external_url(@token_instance.instance) %> target="_blank">
                   View In App <span class="external-token-icon"><%= render BlockScoutWeb.IconsView, "_external_link.html" %></span>
                   </a>
                 </span>

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,10 +102,14 @@ 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(string) when is_binary(string), do: external_url(nil)
+
   def external_url(instance) do
     result =
       if instance.metadata && instance.metadata["external_url"] do
-        instance.metadata["external_url"]
+        instance.metadata["external_url"] |> external_url()
       else
         external_url(nil)
       end

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:198
+#: lib/block_scout_web/views/tokens/instance/overview_view.ex:202
 #, 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:197
+#: lib/block_scout_web/views/tokens/instance/overview_view.ex:201
 #: lib/block_scout_web/views/tokens/overview_view.ex:39
 #: lib/block_scout_web/views/transaction_view.ex:526
 #, 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:198
+#: lib/block_scout_web/views/tokens/instance/overview_view.ex:202
 #, 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:197
+#: lib/block_scout_web/views/tokens/instance/overview_view.ex:201
 #: lib/block_scout_web/views/tokens/overview_view.ex:39
 #: lib/block_scout_web/views/transaction_view.ex:526
 #, 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
@@ -119,4 +119,40 @@ defmodule BlockScoutWeb.Tokens.Instance.OverviewViewTest do
                "https://assets.cargo.build/611a883b0d039100261bfe79/b89cf189-13e9-47ed-b801-a1f6aa15a7bf/a0784ea0-45be-41cd-9cdd-cc40ad20f20d-zombiepngpng.png"
     end
   end
+
+  describe "external_url/1" do
+    test "does not return invalid url scheme" do
+      json = """
+        {
+          "name": "CELO XSS",
+          "image": "https://0-a.nl/nft/nft.jpg",
+          "description": "CELO XSS",
+          "external_url": "javascript:eval(atob('YWxlcnQoIndoYXRzdXAgaXQncyB5YSBib3l5Iik'))"
+        }
+      """
+
+      data = Jason.decode!(json)
+
+      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 = """
+        {
+          "name": "CELO NFT test",
+          "image": "https://0-a.nl/nft/nft.jpg",
+          "description": "CELO NFT test",
+          "external_url": "https://happyland.nft"
+        }
+      """
+
+      data = Jason.decode!(json)
+
+      result = OverviewView.external_url(%{metadata: data})
+
+      assert String.starts_with?(result, "http"), "Valid url should be returned"
+    end
+  end
 end