Hello Juan Hernandez,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/19153
to review the following change.
Change subject: core: Avoid XSS in RedirectServlet
......................................................................
core: Avoid XSS in RedirectServlet
Currently the RedirectServlet composes JavaScript code to show error
messages using text provided by the user in a request parameter. This
text isn't sanitized and thus can be used by maliciuous users to execute
arbitrary JavaScript code. To avoid this situation this patch changes
the servlet so that it doesn't receive any parameter, thus the problem
is completely avoided.
Change-Id: Ie77e6a063e1522b2e108076a240939ca1dae272e
Bug-Url: https://bugzilla.redhat.com/988970
Signed-off-by: Juan Hernandez <[email protected]>
---
M backend/manager/modules/root/src/main/webapp/index.html
D
frontend/wars/rmw-war/src/main/java/org/ovirt/engine/core/redirect/RedirectServlet.java
A
frontend/wars/rmw-war/src/main/java/org/ovirt/engine/core/redirect/ReportsRedirectServlet.java
M frontend/wars/rmw-war/src/main/webapp/WEB-INF/web.xml
4 files changed, 47 insertions(+), 115 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/53/19153/1
diff --git a/backend/manager/modules/root/src/main/webapp/index.html
b/backend/manager/modules/root/src/main/webapp/index.html
index 347976f..5b62254 100644
--- a/backend/manager/modules/root/src/main/webapp/index.html
+++ b/backend/manager/modules/root/src/main/webapp/index.html
@@ -47,7 +47,7 @@
</h2>
<div><a href="UserPortal">User Portal</a></div>
<div><a href="webadmin">Administrator
Portal</a></div>
- <div><a
href="OvirtEngineWeb/RedirectServlet?Page=Reports">Reports Portal</a></div>
+ <div><a
href="OvirtEngineWeb/ReportsRedirectServlet">Reports Portal</a></div>
</div>
</div>
</div>
diff --git
a/frontend/wars/rmw-war/src/main/java/org/ovirt/engine/core/redirect/RedirectServlet.java
b/frontend/wars/rmw-war/src/main/java/org/ovirt/engine/core/redirect/RedirectServlet.java
deleted file mode 100644
index 63bf82e..0000000
---
a/frontend/wars/rmw-war/src/main/java/org/ovirt/engine/core/redirect/RedirectServlet.java
+++ /dev/null
@@ -1,110 +0,0 @@
-package org.ovirt.engine.core.redirect;
-
-import java.io.IOException;
-import java.io.PrintWriter;
-
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
-import org.ovirt.engine.core.bll.interfaces.BackendInternal;
-import org.ovirt.engine.core.common.config.ConfigCommon;
-import org.ovirt.engine.core.common.queries.GetConfigurationValueParameters;
-import org.ovirt.engine.core.common.queries.VdcQueryReturnValue;
-import org.ovirt.engine.core.common.queries.VdcQueryType;
-import org.ovirt.engine.core.common.queries.ConfigurationValues;
-import org.ovirt.engine.core.utils.log.Log;
-import org.ovirt.engine.core.utils.log.LogFactory;
-import org.ovirt.engine.core.utils.ejb.BeanProxyType;
-import org.ovirt.engine.core.utils.ejb.BeanType;
-import org.ovirt.engine.core.utils.ejb.EjbUtils;
-
-public class RedirectServlet extends HttpServlet {
-
- private static Log log = LogFactory.getLog(RedirectServlet.class);
- private static String pagePrefix = "RedirectServlet";
- private static String pageSuffix = "Page";
- private static String errorSuffix = "PageError";
-
- //
***************************************************************************** //
- //
- // IMPORTANT!!!
- //
- // We must use page and error prefix and suffix, if not user can fetch any
- // configuration value from the vdc_option table!
- //
- //
***************************************************************************** //
-
- private String getConfigValue(ConfigurationValues conf) {
- String retVal = null;
- BackendInternal backend = null;
- GetConfigurationValueParameters params = null;
- VdcQueryReturnValue v = null;
-
- try {
- backend = (BackendInternal) EjbUtils.findBean(BeanType.BACKEND,
BeanProxyType.LOCAL);
-
- params = new GetConfigurationValueParameters(conf,
ConfigCommon.defaultConfigurationVersion);
-
- v = backend.runInternalQuery(VdcQueryType.GetConfigurationValue,
params);
- if (v != null) {
- retVal = (v.getSucceeded() && v.getReturnValue() != null &&
!((String)v.getReturnValue()).trim().equals(""))
- ? v.getReturnValue().toString() : null;
- } else {
- log.error("Redirect Servlet: Got NULL from backend.RunQuery!");
- } } catch (Throwable t) {
- log.error("Redirect Servlet: Caught exception while trying to run
query: ", t);
- }
-
- return retVal;
- }
-
- protected void addAlert(PrintWriter out, String message) {
- out.print("<html><body><script>alert('" + message.replace('\'', '"') +
"');window.history.back()</script></body></html>");
- }
-
- protected void doGet(HttpServletRequest request, HttpServletResponse
response)
- throws ServletException, IOException {
-
-
- response.setContentType("text/html");
- PrintWriter out = response.getWriter();
-
- try {
- String page = pagePrefix + request.getParameter("Page") +
pageSuffix;
- if (request.getParameter("Page") == null) {
- addAlert(out, "Page parameter is mandatory");
- }
- else {
- String pageUrl =
getConfigValue(ConfigurationValues.valueOf(page));
- if (pageUrl == null || pageUrl.trim().equals("")) {
- String pageError =
getConfigValue(ConfigurationValues.valueOf(pagePrefix +
request.getParameter("Page") + errorSuffix));
- if (pageError == null) {
- addAlert(out, "Cannot find page: " +
request.getParameter("Page"));
- }
- else {
- addAlert(out, pageError);
- }
- }
- else {
- response.sendRedirect(pageUrl);
- }
- }
- } catch (IllegalArgumentException e) {
- response.setStatus(400);
- addAlert(out, "Page: " + request.getParameter("Page") + " is not a
leagal.");
- } catch (Exception e1) {
- response.setStatus(500);
- log.error("Redirect Servlet: Error", e1);
- } finally {
- out.close();
- }
- log.debug("Health Status servlet: close");
- }
-
- protected void doPost(HttpServletRequest request, HttpServletResponse
response)
- throws ServletException, IOException {
- doGet(request, response);
- }
-}
diff --git
a/frontend/wars/rmw-war/src/main/java/org/ovirt/engine/core/redirect/ReportsRedirectServlet.java
b/frontend/wars/rmw-war/src/main/java/org/ovirt/engine/core/redirect/ReportsRedirectServlet.java
new file mode 100644
index 0000000..db20c18
--- /dev/null
+++
b/frontend/wars/rmw-war/src/main/java/org/ovirt/engine/core/redirect/ReportsRedirectServlet.java
@@ -0,0 +1,42 @@
+package org.ovirt.engine.core.redirect;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.commons.lang.StringUtils;
+import org.ovirt.engine.core.common.config.Config;
+import org.ovirt.engine.core.common.config.ConfigValues;
+
+/**
+ * The purpose of this servlet is to redirect requests to the home page of the
+ * reports application if it is installed or to show an error message to the
+ * user if it isn't. The location of the reports application is taken from the
+ * configuration option <code>RedirectServletPageReports</code>, if it is empty
+ * the servlet assumes that the reports application isn't installed.
+ */
+@SuppressWarnings("serial")
+public class ReportsRedirectServlet extends HttpServlet {
+
+ private void addAlert(PrintWriter out, String message) {
+ out.print("<html><body><script>alert(\"" + message +
"\");window.history.back()</script></body></html>");
+ }
+
+ protected void doGet(HttpServletRequest request, HttpServletResponse
response)
+ throws ServletException, IOException {
+ response.setContentType("text/html");
+ PrintWriter out = response.getWriter();
+ String reportsUrl = Config.<String>
GetValue(ConfigValues.RedirectServletReportsPage);
+ if (StringUtils.isEmpty(reportsUrl)) {
+ addAlert(out, "The reports application isn't installed.");
+ }
+ else {
+ response.sendRedirect(reportsUrl);
+ }
+ }
+
+}
diff --git a/frontend/wars/rmw-war/src/main/webapp/WEB-INF/web.xml
b/frontend/wars/rmw-war/src/main/webapp/WEB-INF/web.xml
index a275d25..a541bea 100644
--- a/frontend/wars/rmw-war/src/main/webapp/WEB-INF/web.xml
+++ b/frontend/wars/rmw-war/src/main/webapp/WEB-INF/web.xml
@@ -21,8 +21,8 @@
<servlet-class>org.ovirt.engine.core.status.HealthStatus</servlet-class>
</servlet>
<servlet>
- <servlet-name>RedirectServlet</servlet-name>
-
<servlet-class>org.ovirt.engine.core.redirect.RedirectServlet</servlet-class>
+ <servlet-name>ReportsRedirectServlet</servlet-name>
+
<servlet-class>org.ovirt.engine.core.redirect.ReportsRedirectServlet</servlet-class>
</servlet>
<servlet>
<servlet-name>ValidateSession</servlet-name>
@@ -48,8 +48,8 @@
<url-pattern>/HealthStatus.aspx</url-pattern>
</servlet-mapping>
<servlet-mapping>
- <servlet-name>RedirectServlet</servlet-name>
- <url-pattern>/RedirectServlet</url-pattern>
+ <servlet-name>ReportsRedirectServlet</servlet-name>
+ <url-pattern>/ReportsRedirectServlet</url-pattern>
</servlet-mapping>
<servlet-mapping>
<servlet-name>ValidateSession</servlet-name>
--
To view, visit http://gerrit.ovirt.org/19153
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie77e6a063e1522b2e108076a240939ca1dae272e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: engine_3.2
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
