Describe the bug
There are a number of instances where it is possible to the tracker to log data passed to it. This is typically seen as bad practice (even when the logs are "innocently" debug logs). Additional light has been shed on this practice with the recent Log4j 2 CVE-2021-44228.
Whilst the Java Tracker doesn't use log4j (at least directly, it uses the sfl4j facade), this CVE has highlighted that logging keys/values is generally something we shouldn't do. debug logging is fine, as that shouldn't be enabled in production and makes debugging far easier when keys/values are listed in the logs. I've listed some debug logging just so another pair of eyes can verify leaving them in seems correct.
To Reproduce
Steps to reproduce the behavior or code snippets that produce the issue.
Expected behavior
No properties passed into the Java Trackers functinos should be logged at info or higher.
Below are the logs I found. I'm not sure they all need fixing as some are debug level but listing here for completeness.
Describe the bug There are a number of instances where it is possible to the tracker to log data passed to it. This is typically seen as bad practice (even when the logs are "innocently" debug logs). Additional light has been shed on this practice with the recent Log4j 2 CVE-2021-44228.
Whilst the Java Tracker doesn't use log4j (at least directly, it uses the sfl4j facade), this CVE has highlighted that logging keys/values is generally something we shouldn't do.
debuglogging is fine, as that shouldn't be enabled in production and makes debugging far easier when keys/values are listed in the logs. I've listed some debug logging just so another pair of eyes can verify leaving them in seems correct.To Reproduce Steps to reproduce the behavior or code snippets that produce the issue.
Expected behavior No properties passed into the Java Trackers functinos should be logged at info or higher.
Below are the logs I found. I'm not sure they all need fixing as some are debug level but listing here for completeness.
https://github.com/snowplow/snowplow-java-tracker/blob/73cd5a496212fd38771c3f0d64a9ecba92f490e2/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java#L123
https://github.com/snowplow/snowplow-java-tracker/blob/73cd5a496212fd38771c3f0d64a9ecba92f490e2/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java#L166
Wrapped with null checks so probably fine, but perhaps printing the value seems pointless. https://github.com/snowplow/snowplow-java-tracker/blob/73cd5a496212fd38771c3f0d64a9ecba92f490e2/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java#L43
Wrapped with null checks so probably fine, but perhaps printing the value seems pointless. https://github.com/snowplow/snowplow-java-tracker/blob/73cd5a496212fd38771c3f0d64a9ecba92f490e2/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java#L47
https://github.com/snowplow/snowplow-java-tracker/blob/73cd5a496212fd38771c3f0d64a9ecba92f490e2/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java#L50
https://github.com/snowplow/snowplow-java-tracker/blob/73cd5a496212fd38771c3f0d64a9ecba92f490e2/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java#L66
https://github.com/snowplow/snowplow-java-tracker/blob/73cd5a496212fd38771c3f0d64a9ecba92f490e2/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java#L89