Skip to content

Commit

Permalink
Merge branch 'master' into B100-ZK-5517
Browse files Browse the repository at this point in the history
  • Loading branch information
jumperchen authored Dec 12, 2023
2 parents 35fcb8b + 1b13b21 commit c35bbf1
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 11 deletions.
9 changes: 7 additions & 2 deletions zk/src/main/java/org/zkoss/zk/ui/http/DHtmlLayoutServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,18 +160,23 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
doGet(request, response);
}


//-- private --//
/**
* Process the request.
* @return false if the page is not found.
* @since 3.0.0
*/
protected boolean process(Session sess, HttpServletRequest request, HttpServletResponse response, String path,
protected boolean process(Session sess, HttpServletRequest request, HttpServletResponse response, String originPath,
boolean bRichlet) throws ServletException, IOException {

// Fix Server-Side Request Forgery (SSRF)
if (!Https.isValidPath(path)) return false;
String path = Https.sanitizePath(originPath);

if (path == null) {
log.warn("The path is sanitized to be null [" + originPath + "]");
return false;
}
final WebApp wapp = sess.getWebApp();
final WebAppCtrl wappc = (WebAppCtrl) wapp;
final Configuration config = wapp.getConfiguration();
Expand Down
9 changes: 7 additions & 2 deletions zk/src/main/java/org/zkoss/zk/ui/http/RichletFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,16 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
chain.doFilter(request, response);
}

protected boolean process(Session sess, HttpServletRequest request, HttpServletResponse response, String path,
protected boolean process(Session sess, HttpServletRequest request, HttpServletResponse response, String originPath,
boolean bRichlet) throws ServletException, IOException {

// Fix Server-Side Request Forgery (SSRF)
if (!Https.isValidPath(path)) return false;
String path = Https.sanitizePath(originPath);

if (path == null) {
log.warn("The path is sanitized to be null [" + originPath + "]");
return false;
}

final WebApp wapp = sess.getWebApp();
final WebAppCtrl wappc = (WebAppCtrl) wapp;
Expand Down
2 changes: 1 addition & 1 deletion zk/src/main/java/org/zkoss/zk/ui/metainfo/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,7 @@ private static String modifyAttrValueIfSimplified(String attrName, String attrVa
if (cc == ',') {
sb.append(cc);
modifiedCommandPropertySb.append(modifyAttrValueIfSimplified0(nm, sb.toString().trim(), paramIndex, isNamedParam));
if (paramIndex != 0 && isNamedParam == false && nm != null) { // named param and un-named parameters together
if (paramIndex != 0 && !isNamedParam && nm != null) { // named param and un-named parameters together
throwCommandSimplifiedErrorUsage();
}
isNamedParam = nm != null;
Expand Down
68 changes: 62 additions & 6 deletions zweb/src/main/java/org/zkoss/web/servlet/http/Https.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@
import java.io.OutputStreamWriter;
import java.io.Reader;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.file.Paths;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -381,16 +384,46 @@ public static final Date toDate(String sdate) throws ParseException {
private static final String PATH_REGEX = "^(/[-\\w:@&?=+,.!/~*'%$_;\\(\\)]*)?$";
private static final Pattern PATH_PATTERN = Pattern.compile(PATH_REGEX);

/**
* Returns the normalized path, or null if it is invalid.
* It is invalid if it is null, or starts with "/" and contains "..".
*
* @since 10.0.0
*/
public static String normalizePath(String path) {
if (path == null) {
return null;
}

List<String> parts = new ArrayList<>();
String[] segments = path.split("/");

for (String segment : segments) {
if (segment.equals("..")) {
if (!parts.isEmpty()) {
parts.remove(parts.size() - 1); // Go up one directory
} else {
// Path is trying to go above the root, which might be a security issue
return null;
}
} else if (!segment.equals(".") && !segment.isEmpty()) {
parts.add(segment); // Add non-empty, non-current directory segments
}
}

return String.join("/", parts);
}

/**
* Returns whether the specified path is valid.
* It is valid if it is null, or starts with "/" and doesn't contain "..".
*
* @since 10.0.0
*/
public static final boolean isValidPath(String path) {
public static boolean isValidPath(String path) {
if (path == null)
return false;
path = Paths.get(path).normalize().toString();
path = normalizePath(path);

if (!PATH_PATTERN.matcher(path).matches()) {
return false;
Expand All @@ -399,11 +432,34 @@ public static final boolean isValidPath(String path) {
return false;
}
final int slash2Count = countToken("//", path);
if (slash2Count > 0) {
return false;
return slash2Count <= 0;
}

/**
* Returns the path of the specified URI, or null if not found.
* It is the same as new URI(uri).getPath(), except that it returns null
* if the path is invalid.
*
* @since 10.0.0
* @see #isValidPath(String)
*/
public static String sanitizePath(String path) {
if (path == null)
return null;
String normalizedPath;
try {
normalizedPath = path.startsWith("/")
? normalizePath(path) :
new URI(path).normalize().toString();
} catch (URISyntaxException e) {
return null;
}

return true;
if (!isValidPath(normalizedPath))
return null;

// Return the sanitized URL path
return normalizedPath;
}

private static int countToken(final String token, final String target) {
Expand Down

0 comments on commit c35bbf1

Please sign in to comment.