If you prefer, you could also make it configurable by adding an option in ConfigurableJohnzonProvider.
The goal is to prevent attacks by HTML injection.

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.

Hello @rmannibucau,

This code

                              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.

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 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>"));

The key there is that there is no security risk returning raw html in JSON, there is no security guarantee returning escaped html in JSON.
The only place it can guarantee the security there is between the json reader and "binder" (react?), and this is where not using unsafe html binding is key and makes you safe.

What will be the code of escapeCauseMyFrontendIsBadAndBindsDataDirectly ?

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.

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: '<>' }

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 &lt; 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.

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.

1 romain

Isnt there anything like ResponseWritterWraper SPI like in JSF? So user can just hookin

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.