Hello Juan Hernandez,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/19152
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.
Signed-off-by: Alexander Wels <aw...@redhat.com>
Signed-off-by: Juan Hernandez <juan.hernan...@redhat.com>
Change-Id: Ie77e6a063e1522b2e108076a240939ca1dae272e
---
D
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/redirect/RedirectServlet.java
A
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/redirect/ReportsRedirectServlet.java
M backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml
M packaging/branding/ovirt.brand/welcome_page.template
4 files changed, 48 insertions(+), 112 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/52/19152/1
diff --git
a/backend/manager/modules/root/src/main/java/org/ovirt/engine/core/redirect/RedirectServlet.java
b/backend/manager/modules/root/src/main/java/org/ovirt/engine/core/redirect/RedirectServlet.java
deleted file mode 100644
index f6e79f0..0000000
---
a/backend/manager/modules/root/src/main/java/org/ovirt/engine/core/redirect/RedirectServlet.java
+++ /dev/null
@@ -1,107 +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.apache.commons.lang.StringUtils;
-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) {
- try {
- BackendInternal backend = (BackendInternal)
EjbUtils.findBean(BeanType.BACKEND, BeanProxyType.LOCAL);
-
- GetConfigurationValueParameters params =
- new GetConfigurationValueParameters(conf,
ConfigCommon.defaultConfigurationVersion);
-
- VdcQueryReturnValue v =
backend.runInternalQuery(VdcQueryType.GetConfigurationValue, params);
- if (v != null) {
- return (v.getSucceeded() && StringUtils.isNotEmpty((String)
v.getReturnValue()))
- ? 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 null;
- }
-
- 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
legal.");
- } catch (Exception e1) {
- response.setStatus(500);
- log.error("Redirect Servlet: Error", e1);
- } finally {
- out.close();
- }
- }
-
- protected void doPost(HttpServletRequest request, HttpServletResponse
response)
- throws ServletException, IOException {
- doGet(request, response);
- }
-}
diff --git
a/backend/manager/modules/root/src/main/java/org/ovirt/engine/core/redirect/ReportsRedirectServlet.java
b/backend/manager/modules/root/src/main/java/org/ovirt/engine/core/redirect/ReportsRedirectServlet.java
new file mode 100644
index 0000000..924fca5
--- /dev/null
+++
b/backend/manager/modules/root/src/main/java/org/ovirt/engine/core/redirect/ReportsRedirectServlet.java
@@ -0,0 +1,43 @@
+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>");
+ }
+
+ @Override
+ 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/backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml
b/backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml
index 5077d61..b7ea8b8 100644
--- a/backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml
+++ b/backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml
@@ -216,12 +216,12 @@
<!-- Redirect servlet (used by the reports integration): -->
<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-mapping>
- <servlet-name>RedirectServlet</servlet-name>
- <url-pattern>/OvirtEngineWeb/RedirectServlet</url-pattern>
+ <servlet-name>ReportsRedirectServlet</servlet-name>
+ <url-pattern>/OvirtEngineWeb/ReportsRedirectServlet</url-pattern>
</servlet-mapping>
<!-- Servlet used to validate sessions (used by the reports
diff --git a/packaging/branding/ovirt.brand/welcome_page.template
b/packaging/branding/ovirt.brand/welcome_page.template
index 1930bef..e349fd4 100644
--- a/packaging/branding/ovirt.brand/welcome_page.template
+++ b/packaging/branding/ovirt.brand/welcome_page.template
@@ -9,6 +9,6 @@
<a
href="/webadmin/webadmin/WebAdmin.html?locale={userLocale}">{portal_admin}</a>
</div>
<div>
- <a href="/OvirtEngineWeb/RedirectServlet?Page=Reports">{portal_reports}</a>
+ <a href="/OvirtEngineWeb/ReportsRedirectServlet">{portal_reports}</a>
</div>
</div>
--
To view, visit http://gerrit.ovirt.org/19152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie77e6a063e1522b2e108076a240939ca1dae272e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
