Skip to content

Commit fee4d08

Browse files
committed
Fixed: Prevent redirect bypass in renderDataResourceAsText URL_RESOURCE path
renderDataResourceAsText() validated the initial URL via checkUrlResourceAllowed() but then fetched it with url.openStream(), which follows HTTP redirects by default. The fix aligns this code path with the already-hardened getDataResourceStream() path: open a URLConnection, disable redirect following with setInstanceFollowRedirects(false), reject any 3xx response, and apply the same configurable connect/read timeouts and response-size cap (BoundedInputStream) used elsewhere. (cherry picked from commit a8d0eba)
1 parent d1bfb92 commit fee4d08

1 file changed

Lines changed: 26 additions & 5 deletions

File tree

applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,12 +1078,33 @@ public static void writeDataResourceText(GenericValue dataResource, String mimeT
10781078

10791079
if (url.getHost() != null) { // is absolute
10801080
checkUrlResourceAllowed(url);
1081-
int c;
1082-
try (InputStream in = url.openStream(); StringWriter sw = new StringWriter()) {
1083-
while ((c = in.read()) != -1) {
1084-
sw.write(c);
1081+
int connectTimeout = (int) UtilProperties.getPropertyNumber("security",
1082+
"content.data.url.resource.connect.timeout", 10000.0);
1083+
int readTimeout = (int) UtilProperties.getPropertyNumber("security",
1084+
"content.data.url.resource.read.timeout", 30000.0);
1085+
long maxResponseSize = (long) UtilProperties.getPropertyNumber("security",
1086+
"content.data.url.resource.max.response.size", (double) (10L * 1024 * 1024));
1087+
URLConnection con = url.openConnection();
1088+
con.setConnectTimeout(connectTimeout);
1089+
con.setReadTimeout(readTimeout);
1090+
// Disable automatic redirect-following to prevent SSRF bypass via redirect to private addresses
1091+
if (con instanceof HttpURLConnection) ((HttpURLConnection) con).setInstanceFollowRedirects(false);
1092+
con.connect();
1093+
// Reject redirects outright; we cannot safely re-validate an arbitrary Location header
1094+
if (con instanceof HttpURLConnection) {
1095+
HttpURLConnection httpCon = (HttpURLConnection) con;
1096+
int responseCode = httpCon.getResponseCode();
1097+
if (responseCode >= 300 && responseCode < 400) {
1098+
httpCon.disconnect();
1099+
throw new GeneralException("URL_RESOURCE request returned a redirect (" + responseCode
1100+
+ "); redirects are not followed for security reasons");
10851101
}
1086-
text = sw.toString();
1102+
}
1103+
try (InputStream limitedIn = BoundedInputStream.builder()
1104+
.setInputStream(con.getInputStream())
1105+
.setMaxCount(maxResponseSize)
1106+
.get()) {
1107+
text = IOUtils.toString(limitedIn, StandardCharsets.UTF_8);
10871108
}
10881109
} else {
10891110
String prefix = DataResourceWorker.buildRequestPrefix(delegator, locale, webSiteId, https);

0 commit comments

Comments
 (0)