Skip to content

Commit

Permalink
Backport for CVE-2023-28708
Browse files Browse the repository at this point in the history
  • Loading branch information
cesarhernandezgt committed Apr 14, 2023
1 parent 8ea8203 commit 30e7e3f
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 29 deletions.
2 changes: 1 addition & 1 deletion build.properties.default
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ version.major=7
version.minor=0
version.build=104
version.patch=0
version.suffix=-SP.7
version.suffix=-SP.8

# ----- Source control flags -----
git.branch=7.0.x
Expand Down
7 changes: 7 additions & 0 deletions java/org/apache/catalina/Globals.java
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,13 @@ public final class Globals {
org.apache.coyote.Constants.SENDFILE_SUPPORTED_ATTR;


/**
* The request attribute that is set to the value of {@code Boolean.TRUE}
* if {@link org.apache.catalina.filters.RemoteIpFilter} determines
* that this request was submitted via a secure channel.
*/
public static final String REMOTE_IP_FILTER_SECURE = "org.apache.catalina.filters.RemoteIpFilter.secure";

/**
* The request attribute that can be used by a servlet to pass
* to the connector the name of the file that is to be served
Expand Down
14 changes: 14 additions & 0 deletions java/org/apache/catalina/connector/Request.java
Original file line number Diff line number Diff line change
Expand Up @@ -3736,6 +3736,20 @@ public void set(Request request, String name, Object value) {
// NO-OP
}
});
specialAttributes.put(Globals.REMOTE_IP_FILTER_SECURE,
new SpecialAttributeAdapter() {
@Override
public Object get(Request request, String name) {
return Boolean.valueOf(request.isSecure());
}

@Override
public void set(Request request, String name, Object value) {
if (value instanceof Boolean) {
request.setSecure(((Boolean) value).booleanValue());
}
}
});

for (SimpleDateFormat sdf : formatsTemplate) {
sdf.setTimeZone(GMT_ZONE);
Expand Down
6 changes: 1 addition & 5 deletions java/org/apache/catalina/filters/RemoteIpFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -564,10 +564,6 @@ public int getServerPort() {
return serverPort;
}

@Override
public boolean isSecure() {
return secure;
}

public void removeHeader(String name) {
Map.Entry<String, List<String>> header = getHeaderEntry(name);
Expand Down Expand Up @@ -608,7 +604,7 @@ public void setScheme(String scheme) {
}

public void setSecure(boolean secure) {
this.secure = secure;
super.getRequest().setAttribute(Globals.REMOTE_IP_FILTER_SECURE, Boolean.valueOf(secure));
}

public void setServerName(String serverName) {
Expand Down
99 changes: 76 additions & 23 deletions test/org/apache/catalina/filters/TestRemoteIpFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,21 @@ public static class MockHttpServlet extends HttpServlet {

private static final long serialVersionUID = 1L;

private transient HttpServletRequest request;

public HttpServletRequest getRequest() {
return request;
}
public String remoteAddr;
public String remoteHost;
public String scheme;
public String serverName;
public int serverPort;
public boolean isSecure;

@Override
public void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
this.request = request;
this.isSecure = request.isSecure();
this.remoteAddr = request.getRemoteAddr();
this.remoteHost = request.getRemoteHost();
this.scheme = request.getScheme();
this.serverName = request.getServerName();
this.serverPort = request.getServerPort();
PrintWriter writer = response.getWriter();

writer.println("request.remoteAddr=" + request.getRemoteAddr());
Expand Down Expand Up @@ -130,16 +136,6 @@ public void setScheme(String scheme) {
getCoyoteRequest().scheme().setString(scheme);
}

@Override
public void setAttribute(String name, Object value) {
getCoyoteRequest().getAttributes().put(name, value);
}

@Override
public Object getAttribute(String name) {
return getCoyoteRequest().getAttributes().get(name);
}

@Override
public String getServerName() {
return "localhost";
Expand Down Expand Up @@ -771,16 +767,73 @@ public void testWithTomcatServer() throws Exception {

// VALIDATE
Assert.assertEquals(HttpURLConnection.HTTP_OK, httpURLConnection.getResponseCode());
HttpServletRequest request = mockServlet.getRequest();
Assert.assertNotNull(request);
//HttpServletRequest request = mockServlet.getRequest();
//Assert.assertNotNull(request);

// VALIDATE X-FORWARDED-FOR
Assert.assertEquals(expectedRemoteAddr, request.getRemoteAddr());
Assert.assertEquals(expectedRemoteAddr, request.getRemoteHost());
Assert.assertEquals(expectedRemoteAddr, mockServlet.remoteAddr);
Assert.assertEquals(expectedRemoteAddr, mockServlet.remoteHost);

// VALIDATE X-FORWARDED-PROTO
Assert.assertTrue(request.isSecure());
Assert.assertEquals("https", request.getScheme());
Assert.assertEquals(443, request.getServerPort());
Assert.assertTrue(mockServlet.isSecure);
Assert.assertEquals("https", mockServlet.scheme);
Assert.assertEquals(443, mockServlet.serverPort);
}

@Test
public void testJSessionIdSecureAttributeMissing() throws Exception {

// mostly default configuration : enable "x-forwarded-proto"
Map<String, String> remoteIpFilterParameter = new HashMap<String, String>();
remoteIpFilterParameter.put("protocolHeader", "x-forwarded-proto");

// SETUP
Tomcat tomcat = getTomcatInstance();
Context root = tomcat.addContext("", TEMP_DIR);

FilterDef filterDef = new FilterDef();
filterDef.getParameterMap().putAll(remoteIpFilterParameter);
filterDef.setFilterClass(RemoteIpFilter.class.getName());
filterDef.setFilterName(RemoteIpFilter.class.getName());

root.addFilterDef(filterDef);

FilterMap filterMap = new FilterMap();
filterMap.setFilterName(RemoteIpFilter.class.getName());
filterMap.addURLPattern("*");
root.addFilterMap(filterMap);

Bug66471Servlet bug66471Servlet = new Bug66471Servlet();

Tomcat.addServlet(root, bug66471Servlet.getClass().getName(), bug66471Servlet);
root.addServletMapping("/test", bug66471Servlet.getClass().getName());

getTomcatInstance().start();

Map<String, List<String>> resHeaders = new HashMap<String, List<String>>();
Map<String, List<String>> reqHeaders = new HashMap<String, List<String>>();
String expectedRemoteAddr = "my-remote-addr";
List<String> forwardedFor = new ArrayList<String>(1);
forwardedFor.add(expectedRemoteAddr);
List<String> forwardedProto = new ArrayList<String>(1);
forwardedProto.add("https");
reqHeaders.put("x-forwarded-for", forwardedFor);
reqHeaders.put("x-forwarded-proto", forwardedProto);

getUrl("http://localhost:" + tomcat.getConnector().getLocalPort() +
"/test", null, reqHeaders, resHeaders);
String setCookie = resHeaders.get("Set-Cookie").get(0);
Assert.assertTrue(setCookie.contains("Secure"));
Assert.assertTrue(bug66471Servlet.isSecure.booleanValue());
}
public static class Bug66471Servlet extends HttpServlet {
private static final long serialVersionUID = 1L;
public Boolean isSecure;

@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
req.getSession();
isSecure = (Boolean) req.getAttribute(Globals.REMOTE_IP_FILTER_SECURE);
}
}
}
5 changes: 5 additions & 0 deletions webapps/docs/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@
Examples. Fix CVE-2022-34305, a low severity XSS vulnerability in the
Form authentication example. (markt)
</fix>
<fix>
<bug>66471</bug>: Fix JSessionId secure attribute missing When
<code>RemoteIpFilter</code> determines that this request was submitted
via a secure channel. (lihan)
</fix>
</changelog>
</subsection>
<subsection name="Coyote">
Expand Down

0 comments on commit 30e7e3f

Please sign in to comment.