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

Default Problem Could Cause Information Disclosure #144

Closed
segocago opened this issue Apr 11, 2022 · 5 comments
Closed

Default Problem Could Cause Information Disclosure #144

segocago opened this issue Apr 11, 2022 · 5 comments
Assignees
Labels
info: workaround available A workaround is available for the issue

Comments

@segocago
Copy link

Expected Behavior

Default error message should not leak any technical information about the infrastructure or the technologies used.

Actual Behaviour

Default Problem built for exceptions that do not extend ThrowableProblem include the exception.getMessage() field in its details part which could carry sensitive information and cause information disclosure.

Steps To Reproduce

Cause a database constraint violation exception through jOOQ (or any other) library which throws an org.postgresql.util.PSQLException exception that includes the query and violated constraint in its message field. defaultProblem() function in ProblemErrorResponseProcessor is sending this message with sql query so the response looks like this:

{
    "type": "about:blank",
    "status": 500,
    "detail": "Internal Server Error: SQL [update \"myTable\" set \"name\" = ? ...... 
}

This is just an example exception. Causing a null pointer exception also sends whole package structure in its error message also causing information disclosure.

Default Problem should either be configurable or send a generic "oops, something went wrong kind of message" response to prevent information disclosure.

Environment Information

  • macOS 11.6
  • Java 17

Example Application

No response

Version

3.4.0

@segocago segocago changed the title Default Problem Could Cause Information Leaks Default Problem Could Cause Information Disclosure Apr 11, 2022
@sdelamo
Copy link
Contributor

sdelamo commented Apr 19, 2022

@hollingsworthd can you evaluate this issue

hollingsworthd pushed a commit that referenced this issue Apr 26, 2022
…d message from returned Problems which fixes #144. Also fix a typo on the field name DEFAULT_STACK_TRACE.
@hollingsworthd
Copy link
Contributor

hollingsworthd commented Apr 26, 2022

Thank you for the details @segocago I was able to reproduce this easily and see the problematic methods you were talking about.

@sdelamo I think the revisions here address the main issue but we could go further and make all the fields of Problem able to be omitted. There could potentially be data leakage from the title for instance. Regarding compatibility, the change here leaves the current behavior in place along with a Javadoc that the default will flip in version 4. My view is that Micronaut is a go-between here potentially to messages from other APIs and that between those other APIs and the end application it seems presumptive for us to omit the details in a minor release. On the other hand, the details field is intended to be the human-readable field so if it breaks anything then the field was perhaps being misused.

sdelamo added a commit that referenced this issue Apr 28, 2022
Unless the exception is of type ThrowableProblem or UnsatisfiedRouteException

This aims to prevent accidental information leakage.

Fixes: #144
@sdelamo
Copy link
Contributor

sdelamo commented Apr 28, 2022

@segocago, thanks for reporting.

I have created a PR to address the issue:

#156

As a workaround, until the patch version is released, you can create a replacement for ProblemErrorProcessor and override defaultProblem method. You can find an example of such a replacement in the linked PR.

@sdelamo sdelamo added the info: workaround available A workaround is available for the issue label Apr 28, 2022
@segocago
Copy link
Author

Thanks for the pull requests.

@sdelamo sdelamo closed this as completed in 8987729 May 2, 2022
@sdelamo
Copy link
Contributor

sdelamo commented May 2, 2022

Thanks for the pull requests.

2.2.3 contains the change already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info: workaround available A workaround is available for the issue
Projects
None yet
3 participants