refactor(xss): missing escape convert on cms resources

Author: mydearxym

lib/groupher_server/cms/job.ex

@@ -16,6 +16,8 @@ defmodule GroupherServer.CMS.Job do
     Tag
   }
 
+  alias Helper.HTML
+
   @timestamps_opts [type: :utc_datetime_usec]
   @required_fields ~w(title company company_logo body digest length)a
   @optional_fields ~w(origial_community_id desc company_link link_addr copy_right salary exp education field finance scale)a
@@ -96,11 +98,6 @@ defmodule GroupherServer.CMS.Job do
     content
     |> validate_length(:title, min: 3, max: 50)
     |> validate_length(:body, min: 3, max: 10_000)
-  end
-
-  @doc false
-  def update_changeset(%Job{} = job, attrs) do
-    job
-    |> cast(attrs, @optional_fields ++ @required_fields)
+    |> HTML.safe_string(:body)
   end
 end

lib/groupher_server/cms/job_comment.ex

@@ -7,6 +7,7 @@ defmodule GroupherServer.CMS.JobComment do
   alias GroupherServer.Accounts
 
   alias GroupherServer.CMS.{Job, JobCommentReply, JobCommentLike, JobCommentDislike}
+  alias Helper.HTML
 
   @required_fields ~w(body author_id job_id floor)a
   @optional_fields ~w(reply_id)a
@@ -46,13 +47,6 @@ defmodule GroupherServer.CMS.JobComment do
     |> foreign_key_constraint(:job_id)
     |> foreign_key_constraint(:author_id)
     |> validate_length(:body, min: 3, max: 2000)
-  end
-
-  @doc false
-  def update_changeset(%JobComment{} = job_comment, attrs) do
-    job_comment
-    |> cast(attrs, @required_fields ++ @optional_fields)
-    |> foreign_key_constraint(:job_id)
-    |> foreign_key_constraint(:author_id)
+    |> HTML.safe_string(:body)
   end
 end

lib/groupher_server/cms/repo.ex

@@ -17,6 +17,8 @@ defmodule GroupherServer.CMS.Repo do
     Tag
   }
 
+  alias Helper.HTML
+
   @timestamps_opts [type: :utc_datetime_usec]
   @required_fields ~w(title owner_name owner_url repo_url desc readme star_count issues_count prs_count fork_count watch_count)a
   @optional_fields ~w(origial_community_id last_sync homepage_url release_tag license)a
@@ -100,13 +102,6 @@ defmodule GroupherServer.CMS.Repo do
     |> validate_length(:title, min: 1, max: 80)
     |> cast_embed(:contributors, with: &RepoContributor.changeset/2)
     |> cast_embed(:primary_language, with: &RepoLang.changeset/2)
-  end
-
-  @doc false
-  def update_changeset(%Repo{} = repo, attrs) do
-    repo
-    |> cast(attrs, @optional_fields ++ @required_fields)
-    |> cast_embed(:contributors, with: &RepoContributor.changeset/2)
-    |> cast_embed(:primary_language, with: &RepoLang.changeset/2)
+    |> HTML.safe_string(:readme)
   end
 end

lib/groupher_server/cms/repo_comment.ex

@@ -13,6 +13,8 @@ defmodule GroupherServer.CMS.RepoComment do
     RepoCommentReply
   }
 
+  alias Helper.HTML
+
   @required_fields ~w(body author_id repo_id floor)a
   @optional_fields ~w(reply_id)a
 
@@ -51,14 +53,6 @@ defmodule GroupherServer.CMS.RepoComment do
     |> foreign_key_constraint(:repo_id)
     |> foreign_key_constraint(:author_id)
     |> validate_length(:body, min: 3, max: 2000)
-  end
-
-  @doc false
-  def update_changeset(%RepoComment{} = repo_comment, attrs) do
-    repo_comment
-    |> cast(attrs, @required_fields ++ @optional_fields)
-    |> validate_length(:body, min: 1)
-    |> foreign_key_constraint(:repo_id)
-    |> foreign_key_constraint(:author_id)
+    |> HTML.safe_string(:body)
   end
 end

lib/groupher_server/cms/video.ex

@@ -93,10 +93,4 @@ defmodule GroupherServer.CMS.Video do
     |> validate_length(:original_author_link, min: 5, max: 200)
     |> validate_length(:link, min: 5, max: 200)
   end
-
-  def update_changeset(%Video{} = video, attrs) do
-    video
-    |> cast(attrs, @optional_fields)
-    |> validate_length(:original_author, min: 3, max: 30)
-  end
 end

lib/groupher_server/cms/video_comment.ex

@@ -13,6 +13,8 @@ defmodule GroupherServer.CMS.VideoComment do
     VideoCommentReply
   }
 
+  alias Helper.HTML
+
   @required_fields ~w(body author_id video_id floor)a
   @optional_fields ~w(reply_id)a
 
@@ -51,14 +53,6 @@ defmodule GroupherServer.CMS.VideoComment do
     |> foreign_key_constraint(:video_id)
     |> foreign_key_constraint(:author_id)
     |> validate_length(:body, min: 3, max: 2000)
-  end
-
-  @doc false
-  def update_changeset(%VideoComment{} = video_comment, attrs) do
-    video_comment
-    |> cast(attrs, @required_fields ++ @optional_fields)
-    |> validate_length(:body, min: 1)
-    |> foreign_key_constraint(:video_id)
-    |> foreign_key_constraint(:author_id)
+    |> HTML.safe_string(:body)
   end
 end

lib/helper/html.ex

@@ -6,10 +6,9 @@ defmodule Helper.HTML do
   import Ecto.Changeset
   alias Phoenix.HTML
 
-  def safe_string(%Ecto.Changeset{} = changeset, field) do
-    case changeset do
-      # %Ecto.Changeset{valid?: true, changes: %{body: val}} ->
-      %Ecto.Changeset{valid?: true, changes: changes} ->
+  def safe_string(%Ecto.Changeset{valid?: true, changes: changes} = changeset, field) do
+    case Map.has_key?(changes, field) do
+      true ->
         changeset
         |> put_change(field, escape_to_safe_string(changes[field]))
 
@@ -18,5 +17,18 @@ defmodule Helper.HTML do
     end
   end
 
+  def safe_string(%Ecto.Changeset{} = changeset, _field), do: changeset
+
+  # def safe_string(%Ecto.Changeset{} = changeset, field) do
+  # case changeset do
+  # %Ecto.Changeset{valid?: true, changes: changes} ->
+  # changeset
+  # |> put_change(field, escape_to_safe_string(changes[field]))
+
+  # _ ->
+  # changeset
+  # end
+  # end
+
   defp escape_to_safe_string(v), do: v |> HTML.html_escape() |> HTML.safe_to_string()
 end

test/groupher_server_web/mutation/cms/job_test.exs

@@ -92,6 +92,21 @@ defmodule GroupherServer.Test.Mutation.Job do
       assert created["id"] == to_string(found.id)
     end
 
+    @tag :wip
+    test "create job should excape xss attracts" do
+      {:ok, user} = db_insert(:user)
+      user_conn = simu_conn(:user, user)
+
+      {:ok, community} = db_insert(:community)
+      job_attr = mock_attrs(:job, %{body: xss_string()})
+
+      variables = job_attr |> Map.merge(%{communityId: community.id}) |> camelize_map_key
+      created = user_conn |> mutation_result(@create_job_query, variables, "createJob")
+      {:ok, job} = ORM.find(CMS.Job, created["id"])
+
+      assert job.body == xss_safe_string()
+    end
+
     test "can create job with tags" do
       {:ok, user} = db_insert(:user)
       user_conn = simu_conn(:user, user)

test/groupher_server_web/mutation/cms/post_comment_test.exs

@@ -48,6 +48,17 @@ defmodule GroupherServer.Test.Mutation.PostComment do
       assert created["id"] == to_string(found.id)
     end
 
+    @tag :wip
+    test "xss comment should be escaped", ~m(user_conn community post)a do
+      variables = %{community: community.raw, thread: "POST", id: post.id, body: xss_string()}
+      created = user_conn |> mutation_result(@create_comment_query, variables, "createComment")
+
+      {:ok, found} = ORM.find(CMS.PostComment, created["id"])
+
+      assert created["id"] == to_string(found.id)
+      assert created["body"] == xss_safe_string()
+    end
+
     test "guest user create comment fails", ~m(guest_conn post community)a do
       variables = %{community: community.raw, thread: "POST", id: post.id, body: "this a comment"}
 

test/groupher_server_web/mutation/cms/post_test.exs

@@ -69,13 +69,13 @@ defmodule GroupherServer.Test.Mutation.Post do
       user_conn = simu_conn(:user, user)
 
       {:ok, community} = db_insert(:community)
-      post_attr = mock_attrs(:post, %{body: "<script>alert(\"hello,world\")</script>"})
+      post_attr = mock_attrs(:post, %{body: xss_string()})
 
       variables = post_attr |> Map.merge(%{communityId: community.id})
       created = user_conn |> mutation_result(@create_post_query, variables, "createPost")
       {:ok, post} = ORM.find(CMS.Post, created["id"])
 
-      assert post.body == "&lt;script&gt;alert(&quot;hello,world&quot;)&lt;/script&gt;"
+      assert post.body == xss_safe_string()
     end
 
     # NOTE: this test is IMPORTANT, cause json_codec: Jason in router will cause