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

Fix: Avoid serializing dangerous types. #1134

Merged
merged 14 commits into from
Jul 23, 2021

Conversation

lucas-zimerman
Copy link
Collaborator

@lucas-zimerman lucas-zimerman commented Jul 20, 2021

This will stop the drop of events and transactions if their content contains a dangerous class type and junk to be serialized.

Changes

  • Replaced property values by null if the type is

Exception
Type

  • Replaced property value by null if the namespace belongs to System.Reflection (avoiding types that leads to infinite loops by self including themselves).

Close #1108, partially close #962.
EIDT: Might also close #950 because it's one of the type of issues that got fixed by removing references to System.Reflection (I wasn't able to reproduce it with TimeZone)

@lucas-zimerman lucas-zimerman added the Bug Something isn't working label Jul 20, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2021

Codecov Report

Merging #1134 (7f052d6) into main (a0b454b) will increase coverage by 0.00%.
The diff coverage is 84.61%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1134   +/-   ##
=======================================
  Coverage   80.79%   80.79%           
=======================================
  Files         200      201    +1     
  Lines        6617     6629   +12     
  Branches     1465     1468    +3     
=======================================
+ Hits         5346     5356   +10     
- Misses        800      801    +1     
- Partials      471      472    +1     
Impacted Files Coverage Δ
.../Sentry.Extensions.Logging/SentryLoggerProvider.cs 97.36% <ø> (ø)
src/Sentry/Internal/SentryJsonConverter.cs 75.00% <75.00%> (ø)
src/Sentry/Internal/Extensions/JsonExtensions.cs 76.47% <100.00%> (+0.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0b454b...7f052d6. Read the comment docs.

src/Sentry/Internal/Extensions/JsonExtensions.cs Outdated Show resolved Hide resolved
src/Sentry/Internal/Extensions/JsonExtensions.cs Outdated Show resolved Hide resolved
src/Sentry/Internal/JsonConverters.cs Outdated Show resolved Hide resolved
@lucas-zimerman lucas-zimerman marked this pull request as ready for review July 21, 2021 16:25
@bruno-garcia bruno-garcia enabled auto-merge (squash) July 23, 2021 17:28
@bruno-garcia bruno-garcia merged commit 3f8ffa1 into main Jul 23, 2021
@bruno-garcia bruno-garcia deleted the fix/system-type-serialization branch July 23, 2021 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
4 participants