Alon Bar-Lev has posted comments on this change. Change subject: userportal,webadmin: fix comment from branding patch ......................................................................
Patch Set 1: (2 inline comments) .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java Line 196: if (path != null) { Line 197: // Make sure path never begins with a '/' Line 198: mergedPath += path.startsWith("/") ? path.substring(1) : path; Line 199: } Line 200: return mergedPath; What is the difference between this and new File(base, path)? As base is from configuration, and path from the user we should check the path to not containing '..' or special characters. Probably the following will work, no? Path basePath = FileSystems.getDefault().getPath(base).normalize(); Path pathPath = FileSystems.getDefault().getPath(base, path).normalize(); if (!pathPath.startsWith(basePath) { throw new SecurityException(...); } Line 201: } .................................................... File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java Line 48: // Get the requested path: Line 49: String path = request.getPathInfo(); Line 50: Line 51: // Locate the requested file: Line 52: String fullPath = getFullPath(brandingManager.getBrandingRootPath().getAbsolutePath(), path); I think that using Path or File all over is best than strings. Line 53: if (fullPath != null) { Line 54: File file = new File(fullPath); Line 55: if (!file.exists() || !file.canRead() || file.isDirectory()) { Line 56: log.error("Unable to retrieve file: " + file.getAbsolutePath() //$NON-NLS-1$ -- To view, visit http://gerrit.ovirt.org/15702 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9fad1cda014713724f294fe8454fb5f36870c137 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches