search

Netflix / conductor

Conductor is a microservices orchestration engine.
Apache License 2.0
12.82k stars 2.34k forks source link

Vulnerability in ClientBase.java CWE 117 Log Forging #1995

Open trayo opened 3 years ago

trayo commented 3 years ago

https://github.com/Netflix/conductor/blob/954c9a8dfd6a478747c8d95d108ebf1162451bb3/client/src/main/java/com/netflix/conductor/client/http/ClientBase.java#L257

I'm creating this issue to track a log forging vulnerability being report by a couple of security scans in ClientBase.java on the line linked to above.

The vulnerability is CWE 117.

aravindanr commented 3 years ago

CWE-117 states,

Data enters an application from an untrusted source.

I am guessing the relevance of that here is that the log prints the response from the server. Is that correct?

trayo commented 3 years ago

CWE-117 states,

Data enters an application from an untrusted source.

I am guessing the relevance of that here is that the log prints the response from the server. Is that correct?

Yes. The fix is generally to have some sort of sanitization like ensuring that there are no /n newlines or /r carriage returns.

aravindanr commented 3 years ago

Understood. We won't be able to get to it anytime soon. If you are inclined, a PR is greatly appreciated.

safat commented 3 years ago

Hello @aravindanr, I would like to work on this.

Hello @trayo, can you tell me which scanner reported this warning and how to use it? I would like to know how I can validate the fix.

trayo commented 3 years ago

Hello @trayo, can you tell me which scanner reported this warning and how to use it? I would like to know how I can validate the fix.

The source code scanners we use are WhiteSource and Fortify. I can't really tell you how to use them because they were set up internally for us and run against our code when we deploy.

safat commented 3 years ago

Thank you @trayo , I will setup WhiteSource or Fortify locally.

safat commented 3 years ago

To update, I've signed up for a trial version of WhiteSource to test the fix, expecting to get the installation files and setup guidelines soon from the WhiteSource.

safat commented 3 years ago

I managed to run a scan in Github using the free trial. But I don't see the CWE-117 to be reported.

https://github.com/safat/conductor/pull/2/checks?check_run_id=1831647898

@trayo, how did you run the scan? I was told that the free trial version report might not be the same as the full-fledged version 😭

trayo commented 3 years ago

I managed to run a scan in Github using the free trial. But I don't see the CWE-117 to be reported.

https://github.com/safat/conductor/pull/2/checks?check_run_id=1831647898

@trayo, how did you run the scan? I was told that the free trial version report might not be the same as the full-fledged version 😭

Sorry @safat that I wasn't clear that my company has the paid version they've set up for us. I might have to tackle this one myself since the trial version doesn't detect it. I'll try and find time in the next week.

Edit: Got severly swept up in priorities at work so I've been unable to look at this issue.