You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@johnzon.apache.org by "gaellalire (via GitHub)" <gi...@apache.org> on 2024/03/14 13:48:53 UTC
[PR] Escape < and > characters [johnzon]
gaellalire opened a new pull request, #122:
URL: https://github.com/apache/johnzon/pull/122
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "gaellalire (via GitHub)" <gi...@apache.org>.
gaellalire commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1997956222
Hello @rmannibucau,
This code
```java
final MyModel object = new MyModel("<script>alert('boom')</script>");
System.out.println("create: " + object + " - " + System.identityHashCode(object));
final Mapper mapper = new MapperBuilder().build();
ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
mapper.writeObject(object, byteArrayOutputStream);
byte[] byteArray = byteArrayOutputStream.toByteArray();
System.out.println("serialize: " + new String(byteArray));
final MyModel otherObject = mapper.readObject(new ByteArrayInputStream(byteArray), MyModel.class);
System.out.println("unserialize: " + otherObject + " - " + System.identityHashCode(otherObject));
```
will produce
```
create: <script>alert('boom')</script> - 366712642
serialize: {"name":"\u003Cscript\u003Ealert('boom')\u003C/script\u003E"}
unserialize: <script>alert('boom')</script> - 1419810764
```
with the patch and not
```
unserialize: \u003Cscript\u003Ealert('boom')\u003C/script\u003E - 1419810764
```
as you maybe have expected.
There is no need to change the read part to be symmetric both '<' and '\u003C' will produce the same java char '<'. It is only another way to print the char in a JSON string.
I was debuging TomEE and I think this is the only place to have a safe response in all our REST methods using JaxRS. If you think there is better place / layer please tell me where. I don't want to escape input parameters, because we will lose ability to access / research objects containing '<' or '>', so for me the response must be escaped and I did not see another place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "gaellalire (via GitHub)" <gi...@apache.org>.
gaellalire commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1997513272
If you prefer, you could also make it configurable by adding an option in ConfigurableJohnzonProvider.
The goal is to prevent attacks by HTML injection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1998499592
ultimately I guess doing a decorator on top of JSON-P to override write methods with string values (unlikely for keys) is possible and already pluggable in johnzon but I would really discourage anything like that and just handle open inputs/forms properly in the backend and the binding in the frontend securely.
Will be way more efficient (perf) and sane (safe + easy to audit) IMHO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1998478259
> Yes because of the number of method I have, I prefer a global solution. Security team don't want script injection, I don't want to change existing behavior.
We likely aim at reaching the same goal.
> You can escape < to < but in that case you will have to ask all of your clients to add unescapeHTML in their code (every methods, every string fields ...).
Depends how you do it but ultimately no, just at the data binding time.
> The beauty here is that this escape is not a real one for JSON ( "\u003C" = "<" : it is just another way to write it) while for an HTML parser ("\u003C" != "<") it will behave differently.
Yes and why we discuss it is that you can still get injections so you just made it harder to understand but you didn't solve the issue you PR for.
> So with that patch, REST clients have nothing to change and security team cannot insert script tags.
Maybe not your team but I know some people and frontend apps who are able to do it without more changes ;).
Long story short: HTML injections can only be handled in the frontend to be safely handled, anything else is just faking a fix.
It is similar to "SQL injection can only be fixed at JDBC layer" (in java but more generally at binding time/protocol abstraction).
All the fixes in WAF to filter SQL injections make it less likely to happen but they can be bypassed and you still get SQL injections.
This is why originally I spoke about fixing the issue at the right layer, JSON can look tempting and I fully understand your reasoning but it does not fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1997968664
> I think I understood, you want to escape after Johnzon finished its serialization. But it is inefficient we will have to parse and rewrite the JSON.
I was more thinking about doing it before:
> new MyModel(escapeCauseMyFrontendIsBadAndBindsDataDirectly("<script>alert('boom')</script>"));
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "gaellalire (via GitHub)" <gi...@apache.org>.
gaellalire commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1997966006
I think I understood, you want to escape after Johnzon finished its serialization. But it is inefficient we will have to parse and rewrite the JSON.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "gaellalire (via GitHub)" <gi...@apache.org>.
gaellalire commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1998060261
> to me it sounds more like you want to sanitize whatever json johnzon produced (from probably untrusted input).
Yes because of the number of method I have, I prefer a global solution. Security team don't want script injection, I don't want to change existing behavior.
You can escape < to < but in that case you will have to ask all of your clients to add unescapeHTML in their code (every methods, every string fields ...).
The beauty here is that this escape is not a real one for JSON ( "\u003C" = "<" : it is just another way to write it) while for an HTML parser ("\u003C" != "<") it will behave differently.
So with that patch, REST clients have nothing to change and security team cannot insert script tags.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1998492600
+1 romain
Isnt there anything like ResponseWritterWraper SPI like in JSF? So user can just hookin
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1997903610
Hi @gaellalire , I don't really get the fix - I understand you want to prevent, let say `<script>alert('boom');</script>` to be in a JSON string but escaping will not be a fix since the fix is in the DOM - or XML document depending where you want the injection - so the fix belong to another layer whatever you do since between johnzon and the next layer you can unescape IMHO.
Side note: if we go with a config we must also ensure the config unescape to have the write/read symmetric at the minimum so will not help your case I fear.
Hope it makes sense.
If I missed a case don't hesitate to give an example/add a test to let me understand more the use case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "jungm (via GitHub)" <gi...@apache.org>.
jungm commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1998029313
to me it sounds more like you want to sanitize whatever json johnzon produced (from probably untrusted input). sanitizing input so you can somewhat safely directly inject it into the DOM is absolutely not an easy task, but I'm also having issues right now understanding how escaping HTML would help you
For example, JS just does this and automatically unescapes again:
```
$ node
Welcome to Node.js v19.7.0.
Type ".help" for more information.
> JSON.parse("{\"value\": \"\u003C\u003E\"}")
{ value: '<>' }
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Escape < and > characters [johnzon]
Posted by "gaellalire (via GitHub)" <gi...@apache.org>.
gaellalire commented on PR #122:
URL: https://github.com/apache/johnzon/pull/122#issuecomment-1997987696
What will be the code of escapeCauseMyFrontendIsBadAndBindsDataDirectly ?
```java
String escapeCauseMyFrontendIsBadAndBindsDataDirectly(String param) {
return param.replaceAll("<", "\u003C").replaceAll(">", "\u003E"); // is like identity, it is doing nothing
}
```
You cannot do anything here if your method take a String and return a String.
We can only patch the serializer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org