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

fix #953 Possible reflective cross site scripting in ConstraintViolations #1001

Merged
merged 1 commit into from Apr 25, 2015

Conversation

@WarFox
Copy link

@WarFox WarFox commented Apr 19, 2015

  • ConstraintViolations should not print the invalid value as it may cause reflective-cross-site scripting issue
  • Fixed all unit tests that expect the (was {invalidValue}) in the violation message.
* ConstraintViolation should not print the invalid value as it may cause reflective-cross-site scripting issue
* Fixed all unit tests that expect the (was {invalidValue}) in the violation message.
@piefel
Copy link

@piefel piefel commented Apr 21, 2015

Since the format() call is also used for the trace logging in JacksonMessageBodyProvider where printing the invalid value is harmless but helpful, I wonder whether there couldn’t be a second format function that would exhibit the classic behaviour.

@carlo-rtr
Copy link
Member

@carlo-rtr carlo-rtr commented Apr 24, 2015

I think we should add a debug log with the invalid value, so it can be debugged on the server side.

I think this would also address @piefel concern without having to maintain a flag or separate methods.

@joschi you commented on #892 which is related, what do you think about this?

@joschi joschi added this to the 0.9.0 milestone Apr 25, 2015
@joschi
Copy link
Member

@joschi joschi commented Apr 25, 2015

I'm all for it. Let's just remove the original, tainted input from the error messages.

joschi added a commit that referenced this pull request Apr 25, 2015
Possible reflective cross site scripting in ConstraintViolation

Fixes #953
@joschi joschi merged commit fe53b65 into dropwizard:master Apr 25, 2015
@WarFox
Copy link
Author

@WarFox WarFox commented Apr 25, 2015

I agree with @carlo-rtr about the debug logs, that could be of help to some projects.

Should we add the debug logs?

@arteam arteam added the bug label Apr 26, 2015
@carlo-rtr
Copy link
Member

@carlo-rtr carlo-rtr commented Apr 27, 2015

I think so :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants