serilog-settings-configuration icon indicating copy to clipboard operation
serilog-settings-configuration copied to clipboard

8.0.1 breaks Serilog.Enrichers.Sensitive "Operators" config, currently failing silently, leaking data

Open scott-r-lindsey opened this issue 1 year ago • 1 comments

Hi there,

Serilog.Enrichers.Sensitive supports masking of custom data types via Masking Operators, but it seems to no longer work after upgrading from 8.0.0 to 8.0.1.

{
  "Serilog": {
    "Using": [
      "Serilog.Enrichers.Sensitive"
    ],
    "Enrich": [
      {
        "Name": "WithSensitiveDataMasking",
        "Args": {
          "options": {
            "MaskValue": "CUSTOM_MASK_FROM_JSON",
            "Operators": [ "MyApplication.Logging.Serilog.MyCustomMaskingOperator, MyAppliation.Logging" ]
          }
        }
      }
    ]
  }
}

This should cause "MyApplication.Logging.Serilog.MyCustomMaskingOperator" to be invoked on each log event to add extra filtering, but after an upgrade to 8.0.1, it instead fails silently.

Fortunately, we spotted this before going to production with current dependencies and we can roll back. However, it seems likely that someone is leaking sensitive data into log files right now.

scott-r-lindsey avatar Aug 08 '24 19:08 scott-r-lindsey

Thanks for the heads-up!

Serilog.Enrichers.Sensitive has a now-obsolete workaround to make its default options constructor callable:

https://github.com/serilog-contrib/Serilog.Enrichers.Sensitive/blob/master/src/Serilog.Enrichers.Sensitive/SensitiveDataEnricherOptions.cs#L24

Although the constructor accepts operators, the value is thrown away.

Now that Serilog.Settings.Configuration can supply operators, it's passed through the constructor and not via the property.

A fix that would support both older and newer versions of Serilog.Settings.Configuration would be to use the values in operators when it's supplied.

The difference in types between the MaskingOperators property and maskingOperators parameter will also likely cause trouble in the future, so changing the constructor version to use the interface type in the same PR might be a good idea.

nblumhardt avatar Aug 08 '24 22:08 nblumhardt