Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sensitive data not scrubbed out of Phoenix.ActionClauseError #477

Closed
woylie opened this issue May 10, 2021 · 5 comments · Fixed by #619
Closed

Sensitive data not scrubbed out of Phoenix.ActionClauseError #477

woylie opened this issue May 10, 2021 · 5 comments · Fixed by #619
Assignees
Milestone

Comments

@woylie
Copy link

woylie commented May 10, 2021

Important Details

How are you running Sentry?

Saas (sentry.io)

Description

Sensitive data is not scrubbed from exception stack traces.

This is related to getsentry/sentry#9309, which didn't get any answers.

Steps to Reproduce

We have a Phoenix application. The endpoint looks something like this:

defmodule MyAppWeb.Endpoint do
  use Sentry.PlugCapture
  use Phoenix.Endpoint, otp_app: :my_app_web

  @scrubbed_param_keys [
    "current_password",
    "email",
    "password",
    "password_confirmation"
  ]

  # ...

  plug Sentry.PlugContext, body_scrubber: &MyAppWeb.Endpoint.body_scrubber/1

  # ...
  plug MyAppWeb.Router

  def body_scrubber(conn) do
    Sentry.PlugContext.scrub_map(conn.params, @scrubbed_param_keys)
  end
end

We found an Phoenix.ActionClauseError logged in Sentry, but the parameters were not scrubbed. The log entry looks like this (irrelevant information redacted for this issue with ...; values we expected to be scrubbed are replaced with UNREDACTED):

Phoenix.ActionClauseError: no function clause matching in MyAppWeb.MyController.update/2

The following arguments were given to MyAppWeb.MyController.update/2:

    # 1
    %Plug.Conn{adapter: {Plug.Cowboy.Conn, :...}, assigns: %{changeset: #Ecto.Changeset<action: nil, changes: %{}, errors: [...], data: #User<>, valid?: false>, ...}, ..., body_params: %{"_method" => "put", "current_password" => "UNREDACTED", "user" => %{"password" => "UNREDACTED", "password_confirmation" => "UNREDACTED"}}, ..., params: %{..., "current_password" => "UNREDACTED", "user" => %{"password" => "UNREDACTED", "password_confirmation" => "UNREDACTED"}}, ...}

    # 2
    %{"_csrf_token" => "...", "_method" => "put", "current_password" => "UNREDACTED", "user" => %{"password" => "UNREDACTED", "password_confirmation" => "UNREDACTED"}}

  File "lib/my_app_web/controllers/my_controller.ex", line 24, in MyAppWeb.MyController.update/2
  File "lib/my_app_web/controllers/my_controller.ex", line 1, in MyAppWeb.MyController.action/2
  File "lib/my_app_web/controllers/my_controller.ex", line 1, in MyAppWeb.MyController.phoenix_controller_pipeline/2
  File "lib/phoenix/router.ex", line 352, in Phoenix.Router.__call__/2
  File "lib/my_app_web/endpoint.ex", line 1, in MyAppWeb.Endpoint.plug_builder_call/2
  File "lib/my_app_web/endpoint.ex", line 1, in MyAppWeb.Endpoint."call (overridable 3)"/2
  File "lib/my_app_web/endpoint.ex", line 1, in MyAppWeb.Endpoint.call/2
  File "lib/phoenix/endpoint/cowboy2_handler.ex", line 65, in Phoenix.Endpoint.Cowboy2Handler.init/4

What you expected to happen

We expected all fields set in @scrubbed_param_keys to be scrubbed from logged exceptions. However, they are sent to Sentry in clear text. Did we miss some configuration parameter or is this a bug?

@getsentry-release
Copy link
Collaborator

Routing to @getsentry/owners-ingest for triage. ⏲️

@untitaker
Copy link
Member

I believe this is best filed against the Ruby SDK.

Note that "this PII did not get scrubbed" can have many different root causes, and in the previous issue you linked it was not even the same SDK that implemented this functionality. There is server-side settings for scrubbing data, but here you clearly configured it in the SDK.

I'm transferring this to sentry-ruby now.

@untitaker untitaker transferred this issue from getsentry/sentry May 10, 2021
@untitaker
Copy link
Member

I just realized this is not Ruby 😅 Transferring to elixir repo.

@untitaker untitaker transferred this issue from getsentry/sentry-ruby May 10, 2021
@mitchellhenke
Copy link
Contributor

Thanks for opening an issue! I will take a look into this

@mitchellhenke
Copy link
Contributor

This is tricky unfortunately. The error message generated by Phoenix includes the sensitive information in an unstructured string by printing the whole conn, which makes it difficult to scrub appropriately. The scrubbing functionality can only operate on params that are already parsed, and that's why the body scrubbing doesn't work as expected.

Phoenix logs this as debug, not error, which suggests to me that it should potentially not be logged by Sentry by default. I'm also guessing that as a debug message, removing the somewhat significant portions of the logs that could contain sensitive information would not be accepted as a change upstream.

Logs
[debug] ** (Phoenix.ActionClauseError) no function clause matching in TripleSentryExceptionWeb.PageController.index/2

The following arguments were given to TripleSentryExceptionWeb.PageController.index/2:

    # 1
    %Plug.Conn{adapter: {Plug.Cowboy.Conn, :...}, assigns: %{}, before_send: [#Function<0.14009385/1 in Plug.CSRFProtection.call/2>, #Function<2.111539819/1 in Phoenix.Controller.fetch_flash/2>, #Function<0.123471702/1 in Plug.Session.before_send/2>, #Function<0.11227428/1 in Plug.Telemetry.call/2>], body_params: %{}, cookies: %{"_bus_kiosk_key" => "SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYRlBaclZDVzhwT2pZejI2SUdSRXVMMVpN.rk8MxxLJcSgP6dty1whmM_1hH9IkSaGmO1tZuS2sRxI", "_properties_key" => "SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYWVUwWTBha1M1WUFYME9FX0xCWVBuQ1dq.mCJiQQu8M2hAsgiVRk2PLsGUmnNlfv_-2kcPrM39UYM", "remember_user_token" => "eyJfcmFpbHMiOnsibWVzc2FnZSI6Ilcxc3hYU3dpSkRKaEpERXdKRVJIWVRCaGRURlhNbms0U0U0d05qRkZOSGRUVG1VaUxDSXhOakEzTWpnd01EQTNMak13T1RreElsMD0iLCJleHAiOiIyMDIxLTEyLTA2VDE4OjQwOjA3LjMwOVoiLCJwdXIiOm51bGx9fQ%3D%3D--3ae6781bcda11277a0e550f11626850a27711237", "sc" => "1hUZKqbjnTY4NTaBHof14blkeFrWEHA74GMXmdE968aut8fgtvFIfjH7eAGAi2cu"}, halted: false, host: "localhost", method: "GET", owner: #PID<0.566.0>, params: %{"password" => "secret"}, path_info: [], path_params: %{}, port: 4000, private: %{TripleSentryExceptionWeb.Router => {[], %{}}, :phoenix_action => :index, :phoenix_controller => TripleSentryExceptionWeb.PageController, :phoenix_endpoint => TripleSentryExceptionWeb.Endpoint, :phoenix_flash => %{}, :phoenix_format => "html", :phoenix_layout => {TripleSentryExceptionWeb.LayoutView, :app}, :phoenix_request_logger => {"request_logger", "request_logger"}, :phoenix_router => TripleSentryExceptionWeb.Router, :phoenix_view => TripleSentryExceptionWeb.PageView, :plug_session => %{}, :plug_session_fetch => :done}, query_params: %{"password" => "secret"}, query_string: "password=secret", remote_ip: {127, 0, 0, 1}, req_cookies: %{"_bus_kiosk_key" => "SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYRlBaclZDVzhwT2pZejI2SUdSRXVMMVpN.rk8MxxLJcSgP6dty1whmM_1hH9IkSaGmO1tZuS2sRxI", "_properties_key" => "SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYWVUwWTBha1M1WUFYME9FX0xCWVBuQ1dq.mCJiQQu8M2hAsgiVRk2PLsGUmnNlfv_-2kcPrM39UYM", "remember_user_token" => "eyJfcmFpbHMiOnsibWVzc2FnZSI6Ilcxc3hYU3dpSkRKaEpERXdKRVJIWVRCaGRURlhNbms0U0U0d05qRkZOSGRUVG1VaUxDSXhOakEzTWpnd01EQTNMak13T1RreElsMD0iLCJleHAiOiIyMDIxLTEyLTA2VDE4OjQwOjA3LjMwOVoiLCJwdXIiOm51bGx9fQ%3D%3D--3ae6781bcda11277a0e550f11626850a27711237", "sc" => "1hUZKqbjnTY4NTaBHof14blkeFrWEHA74GMXmdE968aut8fgtvFIfjH7eAGAi2cu"}, req_headers: [{"accept", "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8"}, {"accept-encoding", "gzip, deflate"}, {"accept-language", "en-US,en;q=0.5"}, {"cache-control", "max-age=0"}, {"connection", "keep-alive"}, {"cookie", "remember_user_token=eyJfcmFpbHMiOnsibWVzc2FnZSI6Ilcxc3hYU3dpSkRKaEpERXdKRVJIWVRCaGRURlhNbms0U0U0d05qRkZOSGRUVG1VaUxDSXhOakEzTWpnd01EQTNMak13T1RreElsMD0iLCJleHAiOiIyMDIxLTEyLTA2VDE4OjQwOjA3LjMwOVoiLCJwdXIiOm51bGx9fQ%3D%3D--3ae6781bcda11277a0e550f11626850a27711237; sc=1hUZKqbjnTY4NTaBHof14blkeFrWEHA74GMXmdE968aut8fgtvFIfjH7eAGAi2cu; _bus_kiosk_key=SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYRlBaclZDVzhwT2pZejI2SUdSRXVMMVpN.rk8MxxLJcSgP6dty1whmM_1hH9IkSaGmO1tZuS2sRxI; _properties_key=SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYWVUwWTBha1M1WUFYME9FX0xCWVBuQ1dq.mCJiQQu8M2hAsgiVRk2PLsGUmnNlfv_-2kcPrM39UYM"}, {"dnt", "1"}, {"host", "localhost:4000"}, {"sec-fetch-dest", "document"}, {"sec-fetch-mode", "navigate"}, {"sec-fetch-site", "none"}, {"upgrade-insecure-requests", "1"}, {"user-agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:90.0) Gecko/20100101 Firefox/90.0"}], request_path: "/", resp_body: nil, resp_cookies: %{}, resp_headers: [{"cache-control", "max-age=0, private, must-revalidate"}, {"x-request-id", "FoG1nAXH6lgX_SwAAALh"}, {"x-frame-options", "SAMEORIGIN"}, {"x-xss-protection", "1; mode=block"}, {"x-content-type-options", "nosniff"}, {"x-download-options", "noopen"}, {"x-permitted-cross-domain-policies", "none"}, {"cross-origin-window-policy", "deny"}], scheme: :http, script_name: [], secret_key_base: :..., state: :unset, status: nil}

    # 2
    %{"password" => "secret"}

    (triple_sentry_exception 0.1.0) lib/triple_sentry_exception_web/controllers/page_controller.ex:5: TripleSentryExceptionWeb.PageController.index(%Plug.Conn{adapter: {Plug.Cowboy.Conn, :...}, assigns: %{}, before_send: [#Function<0.14009385/1 in Plug.CSRFProtection.call/2>, #Function<2.111539819/1 in Phoenix.Controller.fetch_flash/2>, #Function<0.123471702/1 in Plug.Session.before_send/2>, #Function<0.11227428/1 in Plug.Telemetry.call/2>], body_params: %{}, cookies: %{"_bus_kiosk_key" => "SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYRlBaclZDVzhwT2pZejI2SUdSRXVMMVpN.rk8MxxLJcSgP6dty1whmM_1hH9IkSaGmO1tZuS2sRxI", "_properties_key" => "SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYWVUwWTBha1M1WUFYME9FX0xCWVBuQ1dq.mCJiQQu8M2hAsgiVRk2PLsGUmnNlfv_-2kcPrM39UYM", "remember_user_token" => "eyJfcmFpbHMiOnsibWVzc2FnZSI6Ilcxc3hYU3dpSkRKaEpERXdKRVJIWVRCaGRURlhNbms0U0U0d05qRkZOSGRUVG1VaUxDSXhOakEzTWpnd01EQTNMak13T1RreElsMD0iLCJleHAiOiIyMDIxLTEyLTA2VDE4OjQwOjA3LjMwOVoiLCJwdXIiOm51bGx9fQ%3D%3D--3ae6781bcda11277a0e550f11626850a27711237", "sc" => "1hUZKqbjnTY4NTaBHof14blkeFrWEHA74GMXmdE968aut8fgtvFIfjH7eAGAi2cu"}, halted: false, host: "localhost", method: "GET", owner: #PID<0.566.0>, params: %{"password" => "secret"}, path_info: [], path_params: %{}, port: 4000, private: %{TripleSentryExceptionWeb.Router => {[], %{}}, :phoenix_action => :index, :phoenix_controller => TripleSentryExceptionWeb.PageController, :phoenix_endpoint => TripleSentryExceptionWeb.Endpoint, :phoenix_flash => %{}, :phoenix_format => "html", :phoenix_layout => {TripleSentryExceptionWeb.LayoutView, :app}, :phoenix_request_logger => {"request_logger", "request_logger"}, :phoenix_router => TripleSentryExceptionWeb.Router, :phoenix_view => TripleSentryExceptionWeb.PageView, :plug_session => %{}, :plug_session_fetch => :done}, query_params: %{"password" => "secret"}, query_string: "password=secret", remote_ip: {127, 0, 0, 1}, req_cookies: %{"_bus_kiosk_key" => "SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYRlBaclZDVzhwT2pZejI2SUdSRXVMMVpN.rk8MxxLJcSgP6dty1whmM_1hH9IkSaGmO1tZuS2sRxI", "_properties_key" => "SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYWVUwWTBha1M1WUFYME9FX0xCWVBuQ1dq.mCJiQQu8M2hAsgiVRk2PLsGUmnNlfv_-2kcPrM39UYM", "remember_user_token" => "eyJfcmFpbHMiOnsibWVzc2FnZSI6Ilcxc3hYU3dpSkRKaEpERXdKRVJIWVRCaGRURlhNbms0U0U0d05qRkZOSGRUVG1VaUxDSXhOakEzTWpnd01EQTNMak13T1RreElsMD0iLCJleHAiOiIyMDIxLTEyLTA2VDE4OjQwOjA3LjMwOVoiLCJwdXIiOm51bGx9fQ%3D%3D--3ae6781bcda11277a0e550f11626850a27711237", "sc" => "1hUZKqbjnTY4NTaBHof14blkeFrWEHA74GMXmdE968aut8fgtvFIfjH7eAGAi2cu"}, req_headers: [{"accept", "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8"}, {"accept-encoding", "gzip, deflate"}, {"accept-language", "en-US,en;q=0.5"}, {"cache-control", "max-age=0"}, {"connection", "keep-alive"}, {"cookie", "remember_user_token=eyJfcmFpbHMiOnsibWVzc2FnZSI6Ilcxc3hYU3dpSkRKaEpERXdKRVJIWVRCaGRURlhNbms0U0U0d05qRkZOSGRUVG1VaUxDSXhOakEzTWpnd01EQTNMak13T1RreElsMD0iLCJleHAiOiIyMDIxLTEyLTA2VDE4OjQwOjA3LjMwOVoiLCJwdXIiOm51bGx9fQ%3D%3D--3ae6781bcda11277a0e550f11626850a27711237; sc=1hUZKqbjnTY4NTaBHof14blkeFrWEHA74GMXmdE968aut8fgtvFIfjH7eAGAi2cu; _bus_kiosk_key=SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYRlBaclZDVzhwT2pZejI2SUdSRXVMMVpN.rk8MxxLJcSgP6dty1whmM_1hH9IkSaGmO1tZuS2sRxI; _properties_key=SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYWVUwWTBha1M1WUFYME9FX0xCWVBuQ1dq.mCJiQQu8M2hAsgiVRk2PLsGUmnNlfv_-2kcPrM39UYM"}, {"dnt", "1"}, {"host", "localhost:4000"}, {"sec-fetch-dest", "document"}, {"sec-fetch-mode", "navigate"}, {"sec-fetch-site", "none"}, {"upgrade-insecure-requests", "1"}, {"user-agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:90.0) Gecko/20100101 Firefox/90.0"}], request_path: "/", resp_body: nil, resp_cookies: %{}, resp_headers: [{"cache-control", "max-age=0, private, must-reval (truncated)

Ignoring the error is not a satisfying or ideal way to solve this, especially if it's a class of error where you want to be notified. A workaround exists by moving parameter requirements out of the function head into application logic and optionally notifying in there, but that's far more of a hassle.

The options available are not attractive, but avoiding capturing these errors seems like the easiest path.

@whatyouhide whatyouhide changed the title Sensitive data not scrubbed from exception stack traces Sensitive data not scrubbed out of Phoenix.ActionClauseError Aug 31, 2023
@whatyouhide whatyouhide added this to the 9.1.0 milestone Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants