This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch 4.1.x-fixes in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 34b741ccef39acaec50cf6df3ae52677fd2e0642 Author: Naveed Khan <[email protected]> AuthorDate: Thu Oct 29 14:22:54 2026 +0530 restrict jsonp callback names in JsonpInInterceptor (#3628) * restrict jsonp callback names in JsonpInInterceptor * validate the jsonp callback in handleMessage and harden the jsonp output Move the pattern check into JsonpInInterceptor.handleMessage so subclasses overriding getCallbackValue are covered, make the pattern configurable via setCallbackPattern with a 128 character cap, prefix the padded output with /**/ and set X-Content-Type-Options: nosniff on JSONP responses. * make the jsonp padding start configurable and handle a null callback pattern * make the jsonp nosniff header configurable (cherry picked from commit ea2c3ee57ceef29c6d79d609009cc153522e9136) --- .../jaxrs/provider/jsonp/JsonpInInterceptor.java | 25 ++++ .../jsonp/JsonpJaxrsWriterInterceptor.java | 24 +++- .../provider/jsonp/JsonpPreStreamInterceptor.java | 24 +++- .../jaxrs/provider/jsonp/JsonpInterceptorTest.java | 146 ++++++++++++++++++++- .../jaxrs/JAXRSClientServerSpringBookTest.java | 5 +- 5 files changed, 217 insertions(+), 7 deletions(-) diff --git a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInInterceptor.java b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInInterceptor.java index 8ae30d17d33..10179e74799 100644 --- a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInInterceptor.java +++ b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInInterceptor.java @@ -19,6 +19,8 @@ package org.apache.cxf.jaxrs.provider.jsonp; +import java.util.regex.Pattern; + import org.apache.cxf.common.util.StringUtils; import org.apache.cxf.interceptor.Fault; import org.apache.cxf.jaxrs.utils.JAXRSUtils; @@ -35,9 +37,14 @@ public class JsonpInInterceptor extends AbstractPhaseInterceptor<Message> { public static final String CALLBACK_PARAM = "_jsonp"; public static final String CALLBACK_KEY = "JSONP.CALLBACK"; public static final String DEFAULT_CALLBACK_VALUE = "callback"; + // The callback is written verbatim in front of the JSON, so only allow + // letters, digits, '_', '$' and '.' (enough for plain and dotted callback + // names such as angular.callbacks._0) up to a bounded length + public static final String DEFAULT_CALLBACK_PATTERN = "[A-Za-z0-9_$.]{1,128}"; private String callbackParam = CALLBACK_PARAM; private String defaultCallback = DEFAULT_CALLBACK_VALUE; + private Pattern callbackPattern = Pattern.compile(DEFAULT_CALLBACK_PATTERN); private String acceptType; private String mediaType = JSONP_TYPE; @@ -52,6 +59,10 @@ public class JsonpInInterceptor extends AbstractPhaseInterceptor<Message> { public void handleMessage(Message message) throws Fault { String callbackValue = getCallbackValue(message); if (!StringUtils.isEmpty(callbackValue)) { + if (!callbackPattern.matcher(callbackValue).matches()) { + // not a usable callback name, serve plain JSON + return; + } if (getAcceptType() != null) { // may be needed to enforce the selection of // JSON-aware provider @@ -104,4 +115,18 @@ public class JsonpInInterceptor extends AbstractPhaseInterceptor<Message> { public String getDefaultCallback() { return defaultCallback; } + + /** + * Sets the regular expression a callback name must match in full. + * Values which do not match are ignored and the response is plain JSON. + * A null value restores {@link #DEFAULT_CALLBACK_PATTERN}. + */ + public void setCallbackPattern(String callbackPattern) { + this.callbackPattern = Pattern.compile(callbackPattern == null + ? DEFAULT_CALLBACK_PATTERN : callbackPattern); + } + + public String getCallbackPattern() { + return callbackPattern.pattern(); + } } diff --git a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpJaxrsWriterInterceptor.java b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpJaxrsWriterInterceptor.java index cfa8c72d385..e403a0b1bdf 100644 --- a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpJaxrsWriterInterceptor.java +++ b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpJaxrsWriterInterceptor.java @@ -36,7 +36,9 @@ import org.apache.cxf.message.Message; public class JsonpJaxrsWriterInterceptor implements WriterInterceptor { private String mediaType = JsonpInInterceptor.JSONP_TYPE; + private String paddingStart = JsonpPreStreamInterceptor.DEFAULT_PADDING_START; private String paddingEnd = "("; + private boolean addNosniffHeader = true; public JsonpJaxrsWriterInterceptor() { } @@ -47,7 +49,11 @@ public class JsonpJaxrsWriterInterceptor implements WriterInterceptor { if (!StringUtils.isEmpty(callback)) { context.getHeaders().putSingle(Message.CONTENT_TYPE, JAXRSUtils.toMediaType(getMediaType())); - context.getOutputStream().write((callback + getPaddingEnd()).getBytes(StandardCharsets.UTF_8)); + if (isAddNosniffHeader()) { + context.getHeaders().putSingle("X-Content-Type-Options", "nosniff"); + } + context.getOutputStream().write((getPaddingStart() + callback + getPaddingEnd()) + .getBytes(StandardCharsets.UTF_8)); } context.proceed(); } @@ -60,6 +66,14 @@ public class JsonpJaxrsWriterInterceptor implements WriterInterceptor { return mediaType; } + public void setPaddingStart(String paddingStart) { + this.paddingStart = paddingStart; + } + + public String getPaddingStart() { + return paddingStart; + } + public void setPaddingEnd(String paddingEnd) { this.paddingEnd = paddingEnd; } @@ -68,6 +82,14 @@ public class JsonpJaxrsWriterInterceptor implements WriterInterceptor { return paddingEnd; } + public void setAddNosniffHeader(boolean addNosniffHeader) { + this.addNosniffHeader = addNosniffHeader; + } + + public boolean isAddNosniffHeader() { + return addNosniffHeader; + } + protected String getCallbackValue(Message message) { Exchange exchange = message.getExchange(); return (String) exchange.get(JsonpInInterceptor.CALLBACK_KEY); diff --git a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpPreStreamInterceptor.java b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpPreStreamInterceptor.java index 16f77ba7202..d9d3fcafca7 100644 --- a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpPreStreamInterceptor.java +++ b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpPreStreamInterceptor.java @@ -40,9 +40,12 @@ import org.apache.cxf.phase.PhaseInterceptorChain; * Appends the jsonp callback to json responses when the '_jsonp' parameter has been set in the querystring. */ public class JsonpPreStreamInterceptor extends AbstractJsonpOutInterceptor implements WriterInterceptor { + public static final String DEFAULT_PADDING_START = "/**/"; private String mediaType = JsonpInInterceptor.JSONP_TYPE; + private String paddingStart = DEFAULT_PADDING_START; private String paddingEnd = "("; + private boolean addNosniffHeader = true; public JsonpPreStreamInterceptor() { super(Phase.PRE_STREAM); @@ -52,7 +55,7 @@ public class JsonpPreStreamInterceptor extends AbstractJsonpOutInterceptor imple String callback = getCallbackValue(message); if (!StringUtils.isEmpty(callback)) { setContentType(message); - writeValue(message, callback + getPaddingEnd()); + writeValue(message, getPaddingStart() + callback + getPaddingEnd()); } } @@ -64,6 +67,9 @@ public class JsonpPreStreamInterceptor extends AbstractJsonpOutInterceptor imple message.put(Message.PROTOCOL_HEADERS, headers); } headers.put(Message.CONTENT_TYPE, Collections.singletonList(getMediaType())); + if (isAddNosniffHeader()) { + headers.put("X-Content-Type-Options", Collections.singletonList("nosniff")); + } } public void setMediaType(String mediaType) { @@ -74,6 +80,14 @@ public class JsonpPreStreamInterceptor extends AbstractJsonpOutInterceptor imple return mediaType; } + public void setPaddingStart(String paddingStart) { + this.paddingStart = paddingStart; + } + + public String getPaddingStart() { + return paddingStart; + } + public void setPaddingEnd(String paddingEnd) { this.paddingEnd = paddingEnd; } @@ -82,6 +96,14 @@ public class JsonpPreStreamInterceptor extends AbstractJsonpOutInterceptor imple return paddingEnd; } + public void setAddNosniffHeader(boolean addNosniffHeader) { + this.addNosniffHeader = addNosniffHeader; + } + + public boolean isAddNosniffHeader() { + return addNosniffHeader; + } + @Override public void aroundWriteTo(WriterInterceptorContext context) throws IOException, WebApplicationException { handleMessage(PhaseInterceptorChain.getCurrentMessage()); diff --git a/rt/rs/extensions/providers/src/test/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInterceptorTest.java b/rt/rs/extensions/providers/src/test/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInterceptorTest.java index d75bf1cfb18..531b78dbe45 100644 --- a/rt/rs/extensions/providers/src/test/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInterceptorTest.java +++ b/rt/rs/extensions/providers/src/test/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInterceptorTest.java @@ -21,6 +21,9 @@ package org.apache.cxf.jaxrs.provider.jsonp; import java.io.ByteArrayOutputStream; import java.io.OutputStream; +import java.util.Collections; +import java.util.List; +import java.util.Map; import jakarta.ws.rs.core.MediaType; import org.apache.cxf.message.ExchangeImpl; @@ -31,6 +34,7 @@ import org.junit.Before; import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; public class JsonpInterceptorTest { @@ -61,7 +65,10 @@ public class JsonpInterceptorTest { in.handleMessage(message); preStream.handleMessage(message); postStream.handleMessage(message); - assertEquals("myCallback();", bos.toString()); + assertEquals("/**/myCallback();", bos.toString()); + @SuppressWarnings("unchecked") + Map<String, List<String>> headers = (Map<String, List<String>>)message.get(Message.PROTOCOL_HEADERS); + assertEquals(Collections.singletonList("nosniff"), headers.get("X-Content-Type-Options")); } @@ -80,13 +87,146 @@ public class JsonpInterceptorTest { in.handleMessage(message); preStream.handleMessage(message); postStream.handleMessage(message); - assertEquals("myCallback();", bos.toString()); + assertEquals("/**/myCallback();", bos.toString()); } finally { in.setCallbackParam("_jsonp"); } } + @Test + public void testJsonWithoutPaddingStart() throws Exception { + Message message = new MessageImpl(); + message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON); + message.setExchange(new ExchangeImpl()); + message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM + "=" + "myCallback"); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + message.setContent(OutputStream.class, bos); + + // Process the message + preStream.setPaddingStart(""); + in.handleMessage(message); + preStream.handleMessage(message); + postStream.handleMessage(message); + assertEquals("myCallback();", bos.toString()); + } + + @Test + public void testJsonWithoutNosniffHeader() throws Exception { + Message message = new MessageImpl(); + message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON); + message.setExchange(new ExchangeImpl()); + message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM + "=" + "myCallback"); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + message.setContent(OutputStream.class, bos); + + // Process the message + preStream.setAddNosniffHeader(false); + in.handleMessage(message); + preStream.handleMessage(message); + postStream.handleMessage(message); + assertEquals("/**/myCallback();", bos.toString()); + @SuppressWarnings("unchecked") + Map<String, List<String>> headers = (Map<String, List<String>>)message.get(Message.PROTOCOL_HEADERS); + assertNull(headers.get("X-Content-Type-Options")); + } + + @Test + public void testNullCallbackPatternRestoresDefault() throws Exception { + in.setCallbackPattern("[a-z]+"); + in.setCallbackPattern(null); + assertEquals(JsonpInInterceptor.DEFAULT_CALLBACK_PATTERN, in.getCallbackPattern()); + } + + @Test + public void testJsonWithDottedCallback() throws Exception { + Message message = new MessageImpl(); + message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON); + message.setExchange(new ExchangeImpl()); + message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM + "=" + "angular.callbacks._0"); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + message.setContent(OutputStream.class, bos); + + // Process the message + in.handleMessage(message); + preStream.handleMessage(message); + postStream.handleMessage(message); + assertEquals("/**/angular.callbacks._0();", bos.toString()); + } + + @Test + public void testJsonWithCustomCallbackPattern() throws Exception { + Message message = new MessageImpl(); + message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON); + message.setExchange(new ExchangeImpl()); + message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM + "=" + "angular.callbacks._0"); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + message.setContent(OutputStream.class, bos); + + // Process the message + try { + in.setCallbackPattern("[A-Za-z0-9_]{1,32}"); + in.handleMessage(message); + preStream.handleMessage(message); + postStream.handleMessage(message); + assertNull(message.getExchange().get(JsonpInInterceptor.CALLBACK_KEY)); + assertEquals("", bos.toString()); + } finally { + in.setCallbackPattern(JsonpInInterceptor.DEFAULT_CALLBACK_PATTERN); + } + } + + @Test + public void testJsonWithInvalidCallback() throws Exception { + Message message = new MessageImpl(); + message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON); + message.setExchange(new ExchangeImpl()); + message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM + "=" + "alert(document.domain)//"); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + message.setContent(OutputStream.class, bos); + + // Process the message + in.handleMessage(message); + preStream.handleMessage(message); + postStream.handleMessage(message); + assertNull(message.getExchange().get(JsonpInInterceptor.CALLBACK_KEY)); + assertEquals("", bos.toString()); + } + + @Test + public void testJsonWithInvalidCallbackAndJsonpAccept() throws Exception { + Message message = new MessageImpl(); + message.put(Message.ACCEPT_CONTENT_TYPE, JsonpInInterceptor.JSONP_TYPE); + message.setExchange(new ExchangeImpl()); + message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM + "=" + "alert(document.domain)//"); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + message.setContent(OutputStream.class, bos); + + // Process the message: a rejected callback must not fall back to the default one + in.handleMessage(message); + preStream.handleMessage(message); + postStream.handleMessage(message); + assertNull(message.getExchange().get(JsonpInInterceptor.CALLBACK_KEY)); + assertEquals("", bos.toString()); + } + + @Test + public void testJsonWithOverlongCallback() throws Exception { + Message message = new MessageImpl(); + message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON); + message.setExchange(new ExchangeImpl()); + message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM + "=" + "a".repeat(129)); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + message.setContent(OutputStream.class, bos); + + // Process the message + in.handleMessage(message); + preStream.handleMessage(message); + postStream.handleMessage(message); + assertNull(message.getExchange().get(JsonpInInterceptor.CALLBACK_KEY)); + assertEquals("", bos.toString()); + } + @Test public void testJsonWithDefaultPadding() throws Exception { Message message = new MessageImpl(); @@ -99,7 +239,7 @@ public class JsonpInterceptorTest { in.handleMessage(message); preStream.handleMessage(message); postStream.handleMessage(message); - assertEquals("callback();", bos.toString()); + assertEquals("/**/callback();", bos.toString()); } @Test diff --git a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerSpringBookTest.java b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerSpringBookTest.java index 7e39104700a..98bfe27dcec 100644 --- a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerSpringBookTest.java +++ b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerSpringBookTest.java @@ -601,7 +601,8 @@ public class JAXRSClientServerSpringBookTest extends AbstractBusClientServerTest client.query("_jsonp", "callback"); Response r = client.get(); assertEquals("application/x-javascript", r.getMetadata().getFirst("Content-Type")); - assertEquals("callback({\"Book\":{\"id\":123,\"name\":\"CXF in Action\"}});", + assertEquals("nosniff", r.getMetadata().getFirst("X-Content-Type-Options")); + assertEquals("/**/callback({\"Book\":{\"id\":123,\"name\":\"CXF in Action\"}});", IOUtils.readStringFromStream((InputStream)r.getEntity())); } @@ -615,7 +616,7 @@ public class JAXRSClientServerSpringBookTest extends AbstractBusClientServerTest Response r = client.get(); assertEquals("application/x-javascript", r.getMetadata().getFirst("Content-Type")); String response = IOUtils.readStringFromStream((InputStream)r.getEntity()); - assertTrue(response.startsWith("callback({\"class\":\"org.apache.cxf.systest.jaxrs.Book\",")); + assertTrue(response.startsWith("/**/callback({\"class\":\"org.apache.cxf.systest.jaxrs.Book\",")); assertTrue(response.endsWith("});")); assertTrue(response.contains("\"id\":123")); assertTrue(response.contains("\"name\":\"CXF in Action\""));
