Bad sonar security rating

[atomfrede]

Overview of the issue

Because of a change in the sonar rulesets we have now just a B in terms of security in our sample application.
The issue is imho not severe (maybe we can also exclude/ignore it), but we should do something about the red quality gate.

• Checking this box is mandatory (this is just to show you read everything)

[atomfrede]

I tried to forge some logs and succeeded. Running locally you can easily see the entry has been forged as it is not colored, but inside a log file or some other tool that would not be possible. So I tried OWASP easpi to encode it, but didn't have something working here.

[atomfrede]

I have managed to configured https://github.com/javabeanz/owasp-security-logging with the owasp filter documented here https://github.com/javabeanz/owasp-security-logging/wiki/Log-Forging

It looks like this. Maybe we should only enabled it in production as the nice "table like" log gets broken of course. You can see how the line break is replaced by the owasp clrf filter:

Basically this requires defining the clrf filter in logback-spring.xml and setting logging.pattern.console via regular properties using the clrf filter. With this I would say we are doing fine against log forging and could even ignore the sonar warnings regarding log forging. WDYT?

[github-actions]

This issue is stale because it has been open 30 days with no activity.
Our core developers tend to be more verbose on denying. If there is no negative comment, possibly this feature will be accepted.
We are accepting PRs 😃.
Comment or this will be closed in 7 days

[atomfrede]

Keep it open

[github-actions]

This issue is stale because it has been open 30 days with no activity.
Our core developers tend to be more verbose on denying. If there is no negative comment, possibly this feature will be accepted.
We are accepting PRs 😃.
Comment or this will be closed in 7 days

[atomfrede]

Keep it open

[nomuna]

Just cloned the source code for the project and ran a sonar analysis with sonarqube-9.0.1.46107 for windows x64 with the defaults/ sonar quality gates. I don't see any security issues... Am I missing something? Is the project on sonar cloud? If it is could you provide the url?

[atomfrede]

It's the generated sample app not the generator code itself. It is on sonar cloud here https://sonarcloud.io/dashboard?id=jhipster-sample-application

[nomuna]

It's the generated sample app not the generator code itself. It is on sonar cloud here https://sonarcloud.io/dashboard?id=jhipster-sample-application

So it has nothing to do with the code under https://github.com/jhipster/jhipster-sample-app ?

[atomfrede]

It is exactly that project, you are right.

[atomfrede]

But in fact the rating has changed from F to B compared to the screenshots. The log forging is now minor as it seems

[nomuna]

But in fact the rating has changed from F to B compared to the screenshots. The log forging is now minor as it seems

It is seriously weird... The scan I performed is a couple of hrs old and the code was checked out may be at the same time.
I don't see those log forging vulnerabilities. Was there a version change and may be they changed their quality gates? Is it possible?

[nomuna]

BankAccountQueryService class is neither in master nor in main branch. I think the code comes from a different repository.
Or very old and the class in question was deleted long time ago...

[github-actions]

This issue is stale because it has been open 30 days with no activity.
Our core developers tend to be more verbose on denying. If there is no negative comment, possibly this feature will be accepted.
We are accepting PRs 😃.
Comment or this will be closed in 7 days

[atomfrede]

Keep it open

[RobertBurrellDonkin]

I think that https://sonarcloud.io/dashboard?id=jhipster-sample-application is failing now just on code coverage.

What am I missing...?

[nomuna]

I think that https://sonarcloud.io/dashboard?id=jhipster-sample-application is failing now just on code coverage.

What am I missing...?

As I have pointed out in my last message the project has moved on, some classes were deleted and refactored so that the screenshot used as info for the issue is no longer relevant. The devs refuse to accept it apparently... :D IMHO the issue should be closed.

[github-actions]

This issue is stale because it has been open 30 days with no activity.
Our core developers tend to be more verbose on denying. If there is no negative comment, possibly this feature will be accepted.
We are accepting PRs 😃.
Comment or this will be closed in 7 days

[pascalgrimaud]

Probably it's because the Sonar analysis is broken since June:

[atomfrede]

Coming back to this. The issue still exists, but the vulnerabilities are now minor (B). Nevertheless log forging is still possible, while I suppose a lot of people are using json based/structured logging instead of plain pattern layouts, not sure we must do something about it. We could document it more clearly in "going to production" and ignore the sonar issues.
In addition we may also provide json log configuration (by default) for production?