Merge branch 'master' into production

OpenOLAT · May 12, 2014 · 7dc59c6 · 7dc59c6
2 parents 3527d7d + ef70a97
commit 7dc59c6
Show file tree

Hide file tree

Showing 19 changed files with 60 additions and 338 deletions.
diff --git a/...-engine/src/main/java/uk/ac/ed/ph/qtiworks/web/authn/SystemUserAuthenticationServlet.java b/...-engine/src/main/java/uk/ac/ed/ph/qtiworks/web/authn/SystemUserAuthenticationServlet.java
@@ -145,12 +145,14 @@ protected SystemUser tryAuthentication(final String loginName, final String pass
         final SystemUser user = systemUserDao.findByLoginName(loginName);
         final String badDetails = "Sorry, your login details were not correct. Please try again.";
         if (user==null) {
+            logger.debug("System User {} does not exist", loginName);
             errors.add(badDetails);
             return null;
         }
         /* Then check password */
         final String passwordDigest = ServiceUtilities.computePasswordDigest(user.getPasswordSalt(), password);
         if (!passwordDigest.equals(user.getPasswordDigest())) {
+            logger.debug("Password mismatch for System User {}", loginName);
             errors.add(badDetails);
             return null;
         }

diff --git a/qtiworks-engine/src/main/java/uk/ac/ed/ph/qtiworks/web/view/ElFunctions.java b/qtiworks-engine/src/main/java/uk/ac/ed/ph/qtiworks/web/view/ElFunctions.java
@@ -100,12 +100,6 @@ public static String formatDayDateAndTime(final Date time) {
         return time!=null ? ViewUtilities.getDayDateAndTimeFormat().format(time) : "";
     }
 
-    public static String dumpObject(final Object object) {
-        return escapeXml(ObjectDumper.dumpObject(object, DumpMode.DEEP));
-    }
-
-    //-------------------------------------------------
-
     /* NB: We prefer String over URI here as it's more general, and takes advantage of
      * stringification within the JSTL.
      */
@@ -144,6 +138,12 @@ public static String formatTestPlanNodeKey(final TestPlanNodeKey testPlanNodeKey
         return resultBuilder.toString();
     }
 
+    //-------------------------------------------------
+
+    public static String dumpObject(final Object object) {
+        return escapeXml(ObjectDumper.dumpObject(object, DumpMode.DEEP));
+    }
+
     private static String escapeXml(final String rawString) {
         return XmlEscapers.xmlContentEscaper().escape(rawString);
     }

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/login/systemUserLogin.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/login/systemUserLogin.jsp
@@ -25,7 +25,7 @@ All Rights Reserved
     <c:if test="${!empty errors}">
       <ul class="formErrors">
         <c:forEach var="e" items="${errors}">
-          <li>${e}</li>
+          <li>${fn:escapeXml(e)}</li>
         </c:forEach>
       </ul>
     </c:if>
@@ -35,7 +35,7 @@ All Rights Reserved
         <label for="userId">Login ID:</label>
       </div>
       <div class="grid_2">
-        <input size="16" id="loginName" name="loginName" type="text" tabindex="1" value="${loginName}"/>
+        <input size="16" id="loginName" name="loginName" type="text" tabindex="1" value="${fn:escapeXml(loginName)}"/>
       </div>
     </div>
     <div class="clear"></div>
@@ -44,7 +44,7 @@ All Rights Reserved
         <label for="password">Password:</label>
       </div>
       <div class="grid_10">
-        <input size="16" id="password" name="password" type="password" tabindex="2" value="${password}" />
+        <input size="16" id="password" name="password" type="password" tabindex="2" value="${fn:escapeXml(password)}" />
       </div>
     </div>
     <div class="clear"></div>

diff --git a/...ks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/assessmentOutcomesSettingsForm.jsp b/...ks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/assessmentOutcomesSettingsForm.jsp
@@ -56,7 +56,7 @@ outcomeDeclarationList
             <div class="grid_3">
               <form:select path="resultOutcomeIdentifier" id="resultOutcomeIdentifier">
                 <c:forEach var="outcomeDeclaration" items="${outcomeDeclarationList}">
-                  <form:option value="${outcomeDeclaration.identifier}"/>
+                  <form:option value="${fn:escapeXml(outcomeDeclaration.identifier)}"/>
                 </c:forEach>
               </form:select>
             </div>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/createDeliveryForm.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/createDeliveryForm.jsp
@@ -70,7 +70,7 @@ deliveryTemplate
               <li>
                 <input type="radio" id="dsid${ds.id}" name="dsid" value="${ds.id}"${checked ? ' checked="checked"' : ''} />
                 <label for="dsid${ds.id}" class="dsTitle">
-                  ${ds.title}
+                  ${fn:escapeXml(ds.title)}
                 </label>
                 <c:if test="${ds['class'].simpleName=='ItemDeliverySettings' && !empty ds.prompt}">
                   <div class="dsPrompt">${fn:escapeXml(utils:trimSentence(ds.prompt, 200))}</div>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/editDeliveryForm.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/editDeliveryForm.jsp
@@ -72,7 +72,7 @@ deliveryTemplate
               <li>
                 <input type="radio" id="dsid${ds.id}" name="dsid" value="${ds.id}"${checked ? ' checked="checked"' : ''} />
                 <label for="dsid${ds.id}" class="dsTitle">
-                  ${ds.title}
+                  ${fn:escapeXml(ds.title)}
                 </label>
                 <c:if test="${ds['class'].simpleName=='ItemDeliverySettings' && !empty ds.prompt}">
                   <div class="dsPrompt">${fn:escapeXml(utils:trimSentence(ds.prompt, 200))}</div>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/listCandidateSessions.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/listCandidateSessions.jsp
@@ -64,7 +64,7 @@ candidateSessionListRouting (xid -> action -> URL)
             <c:if test="${!empty candidateSessionSummaryMetadata.lisResultOutcomeIdentifier}">
               <c:choose>
                 <c:when test="${rowCount > 0}">
-                  <th>${candidateSessionSummaryMetadata.lisResultOutcomeIdentifier} Value</th>
+                  <th>${fn:escapeXml(candidateSessionSummaryMetadata.lisResultOutcomeIdentifier)} Value</th>
                   <th>Normalized Score</th>
                   <th>Reporting Status</th>
                 </c:when>
@@ -84,16 +84,16 @@ candidateSessionListRouting (xid -> action -> URL)
                     <a href="${utils:escapeLink(candidateSessionListRouting[row.sessionId]['show'])}">${row.sessionId}</a>
                   </td>
                   <td align="center"><c:out value="${utils:formatDateAndTime(row.launchTime)}"/></td>
-                  <td align="center">${row.sessionStatusMessage}</td>
+                  <td align="center"><c:out value="${row.sessionStatusMessage}"/></td>
                   <td><c:out value="${row.firstName}"/></td>
                   <td><c:out value="${row.lastName}"/></td>
                   <td><c:out value="${row.emailAddress}"/></td>
                   <c:if test="${!empty candidateSessionSummaryMetadata.lisResultOutcomeIdentifier}">
                     <c:choose>
                       <c:when test="${!empty row.lisResultOutcomeValue}">
                         <td align="center"><c:out value="${row.lisResultOutcomeValue}"/></td>
-                        <td align="center">${row.lisScore}</td>
-                        <td align="center">${row.lisReportingStatusMessage}</td>
+                        <td align="center"><c:out value="${row.lisScore}"/></td>
+                        <td align="center"><c:out value="${row.lisReportingStatusMessage}"/></td>
                       </c:when>
                       <c:otherwise>
                         <td align="center">(Not Available)</td>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/showAssessment.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/showAssessment.jsp
@@ -112,7 +112,7 @@ Shows information about a particular Assessment
           <div class="value">
             <c:choose>
               <c:when test="${!empty assessment.ltiResultOutcomeIdentifier}">
-                Reporting outcome <code>${assessment.ltiResultOutcomeIdentifier}</code>
+                Reporting outcome <code>${fn:escapeXml(assessment.ltiResultOutcomeIdentifier)}</code>
                 with range [${assessment.ltiResultMinimum}..${assessment.ltiResultMaximum}]
               </c:when>
               <c:otherwise>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/showCandidateSession.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/showCandidateSession.jsp
@@ -15,7 +15,6 @@ candidateEventSummaryDataList
 <%@ include file="/WEB-INF/jsp/includes/pageheader.jspf" %>
 <c:set var="candidateSessionSummaryMetadata" value="${candidateSessionSummaryReport.candidateSessionSummaryMetadata}"/>
 <c:set var="candidateSessionSummaryData" value="${candidateSessionSummaryReport.candidateSessionSummaryData}"/>
-<c:set var="assessmentResultXml" value="${candidateSessionSummaryReport.assessmentResultXml}"/>
 <page:page title="Candidate Session Management">
 
   <header class="actionHeader">
@@ -63,8 +62,8 @@ candidateEventSummaryDataList
   <c:if test="${!empty candidateSessionSummaryMetadata.lisResultOutcomeIdentifier}">
     <div class="grid_4">
       <div class="infoBox">
-        <div class="cat">LTI Result Outcome Variable (${candidateSessionSummaryMetadata.lisResultOutcomeIdentifier})</div>
-        <div class="value">${candidateSessionSummaryData.lisResultOutcomeValue}</div>
+        <div class="cat">LTI Result Outcome Variable (${fn:escapeXml(candidateSessionSummaryMetadata.lisResultOutcomeIdentifier)})</div>
+        <div class="value">${fn:escapeXml(candidateSessionSummaryData.lisResultOutcomeValue)}</div>
       </div>
     </div>
     <div class="grid_4">
@@ -137,16 +136,16 @@ candidateEventSummaryDataList
       <c:if test="${numericOutcomeCount > 0}">
         <c:forEach var="index" begin="0" end="${numericOutcomeCount-1}">
           <tr>
-            <td>${candidateSessionSummaryMetadata.numericOutcomeIdentifiers[index]}</td>
-            <td>${candidateSessionSummaryData.numericOutcomeValues[index]}</td>
+            <td>${fn:escapeXml(candidateSessionSummaryMetadata.numericOutcomeIdentifiers[index])}</td>
+            <td>${fn:escapeXml(candidateSessionSummaryData.numericOutcomeValues[index])}</td>
           </tr>
         </c:forEach>
       </c:if>
       <c:if test="${otherOutcomeCount > 0}">
         <c:forEach var="index" begin="0" end="${otherOutcomeCount-1}">
           <tr>
-            <td>${candidateSessionSummaryMetadata.otherOutcomeIdentifiers[index]}</td>
-            <td>${candidateSessionSummaryData.otherOutcomeValues[index]}</td>
+            <td>${fn:escapeXml(candidateSessionSummaryMetadata.otherOutcomeIdentifiers[index])}</td>
+            <td>${fn:escapeXml(candidateSessionSummaryData.otherOutcomeValues[index])}</td>
           </tr>
         </c:forEach>
       </c:if>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/showCandidateSessionEvents.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/showCandidateSessionEvents.jsp
@@ -15,7 +15,6 @@ candidateEventSummaryDataList
 <%@ include file="/WEB-INF/jsp/includes/pageheader.jspf" %>
 <c:set var="candidateSessionSummaryMetadata" value="${candidateSessionSummaryReport.candidateSessionSummaryMetadata}"/>
 <c:set var="candidateSessionSummaryData" value="${candidateSessionSummaryReport.candidateSessionSummaryData}"/>
-<c:set var="assessmentResultXml" value="${candidateSessionSummaryReport.assessmentResultXml}"/>
 <page:page title="Candidate Session Activity Log">
 
   <header class="actionHeader">

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/showDelivery.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/instructor/showDelivery.jsp
@@ -121,8 +121,8 @@ Shows a Delivery
     </p>
     <ul>
       <li><b>Launch URL</b>: ${fn:escapeXml(deliveryRouting['ltiLaunch'])}</li>
-      <li><b>Key</b>: <code>${delivery.id}X${delivery.ltiConsumerKeyToken}</code></li>
-      <li><b>Secret</b>: <code>${delivery.ltiConsumerSecret}</code></li>
+      <li><b>Key</b>: <code>${delivery.id}X${fn:escapeXml(delivery.ltiConsumerKeyToken)}</code></li>
+      <li><b>Secret</b>: <code>${fn:escapeXml(delivery.ltiConsumerSecret)}</code></li>
     </ul>
   </c:if>
   <div class="floatRight scary actions">

diff --git a/...ngine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/assessmentOutcomesSettingsForm.jsp b/...ngine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/assessmentOutcomesSettingsForm.jsp
@@ -52,7 +52,7 @@ outcomeDeclarationList
             <div class="grid_3">
               <form:select path="resultOutcomeIdentifier" id="resultOutcomeIdentifier">
                 <c:forEach var="outcomeDeclaration" items="${outcomeDeclarationList}">
-                  <form:option value="${outcomeDeclaration.identifier}"/>
+                  <form:option value="${fn:escapeXml(outcomeDeclaration.identifier)}"/>
                 </c:forEach>
               </form:select>
             </div>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/listCandidateSessions.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/listCandidateSessions.jsp
@@ -56,7 +56,7 @@ candidateSessionListRouting (xid -> action -> URL)
             <c:if test="${!empty candidateSessionSummaryMetadata.lisResultOutcomeIdentifier}">
               <c:choose>
                 <c:when test="${rowCount > 0}">
-                  <th>${candidateSessionSummaryMetadata.lisResultOutcomeIdentifier} Value</th>
+                  <th>${fn:escapeXml(candidateSessionSummaryMetadata.lisResultOutcomeIdentifier)} Value</th>
                   <th>Normalized Score</th>
                   <th>Reporting Status</th>
                 </c:when>
@@ -76,16 +76,16 @@ candidateSessionListRouting (xid -> action -> URL)
                     <a href="${utils:escapeLink(candidateSessionListRouting[row.sessionId]['show'])}">${row.sessionId}</a>
                   </td>
                   <td align="center"><c:out value="${utils:formatDateAndTime(row.launchTime)}"/></td>
-                  <td align="center">${row.sessionStatusMessage}</td>
+                  <td align="center"><c:out value="${row.sessionStatusMessage}"/></td>
                   <td><c:out value="${row.firstName}"/></td>
                   <td><c:out value="${row.lastName}"/></td>
                   <td><c:out value="${row.emailAddress}"/></td>
                   <c:if test="${!empty candidateSessionSummaryMetadata.lisResultOutcomeIdentifier}">
                     <c:choose>
                       <c:when test="${!empty row.lisResultOutcomeValue}">
                         <td align="center"><c:out value="${row.lisResultOutcomeValue}"/></td>
-                        <td align="center">${row.lisScore}</td>
-                        <td align="center">${row.lisReportingStatusMessage}</td>
+                        <td align="center"><c:out value="${row.lisScore}"/></td>
+                        <td align="center"><c:out value="${row.lisReportingStatusMessage}"/></td>
                       </c:when>
                       <c:otherwise>
                         <td align="center">(Not Available)</td>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/resourceDashboard.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/resourceDashboard.jsp
@@ -105,7 +105,7 @@ LTI resource dashboard (after domain-level launch)
           <div class="value">
             <c:choose>
               <c:when test="${!empty thisAssessment.ltiResultOutcomeIdentifier}">
-                Reporting outcome <code>${thisAssessment.ltiResultOutcomeIdentifier}</code>
+                Reporting outcome <code>${fn:escapeXml(thisAssessment.ltiResultOutcomeIdentifier)}</code>
                 with range [${thisAssessment.ltiResultMinimum}..${thisAssessment.ltiResultMaximum}]
               </c:when>
               <c:otherwise>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/showAssessment.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/showAssessment.jsp
@@ -111,7 +111,7 @@ Shows information about a particular Assessment
           <div class="value">
             <c:choose>
               <c:when test="${!empty assessment.ltiResultOutcomeIdentifier}">
-                Reporting outcome <code>${assessment.ltiResultOutcomeIdentifier}</code>
+                Reporting outcome <code>${fn:escapeXml(assessment.ltiResultOutcomeIdentifier)}</code>
                 with range [${assessment.ltiResultMinimum}..${assessment.ltiResultMaximum}]
               </c:when>
               <c:otherwise>

diff --git a/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/showCandidateSession.jsp b/qtiworks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/showCandidateSession.jsp
@@ -15,7 +15,6 @@ candidateEventSummaryDataList
 <%@ include file="/WEB-INF/jsp/includes/pageheader.jspf" %>
 <c:set var="candidateSessionSummaryMetadata" value="${candidateSessionSummaryReport.candidateSessionSummaryMetadata}"/>
 <c:set var="candidateSessionSummaryData" value="${candidateSessionSummaryReport.candidateSessionSummaryData}"/>
-<c:set var="assessmentResultXml" value="${candidateSessionSummaryReport.assessmentResultXml}"/>
 <page:ltipage title="Candidate Session Management">
 
   <header class="actionHeader">
@@ -48,8 +47,8 @@ candidateEventSummaryDataList
   <c:if test="${!empty candidateSessionSummaryMetadata.lisResultOutcomeIdentifier}">
     <div class="grid_4">
       <div class="infoBox">
-        <div class="cat">LTI Result Outcome Variable (${candidateSessionSummaryMetadata.lisResultOutcomeIdentifier})</div>
-        <div class="value">${candidateSessionSummaryData.lisResultOutcomeValue}</div>
+        <div class="cat">LTI Result Outcome Variable (${fn:escapeXml(candidateSessionSummaryMetadata.lisResultOutcomeIdentifier)})</div>
+        <div class="value">${fn:escapeXml(candidateSessionSummaryData.lisResultOutcomeValue)}</div>
       </div>
     </div>
     <div class="grid_4">
@@ -127,16 +126,16 @@ candidateEventSummaryDataList
       <c:if test="${numericOutcomeCount > 0}">
         <c:forEach var="index" begin="0" end="${numericOutcomeCount-1}">
           <tr>
-            <td>${candidateSessionSummaryMetadata.numericOutcomeIdentifiers[index]}</td>
-            <td>${candidateSessionSummaryData.numericOutcomeValues[index]}</td>
+            <td>${fn:escapeXml(candidateSessionSummaryMetadata.numericOutcomeIdentifiers[index])}</td>
+            <td>${fn:escapeXml(candidateSessionSummaryData.numericOutcomeValues[index])}</td>
           </tr>
         </c:forEach>
       </c:if>
       <c:if test="${otherOutcomeCount > 0}">
         <c:forEach var="index" begin="0" end="${otherOutcomeCount-1}">
           <tr>
-            <td>${candidateSessionSummaryMetadata.otherOutcomeIdentifiers[index]}</td>
-            <td>${candidateSessionSummaryData.otherOutcomeValues[index]}</td>
+            <td>${fn:escapeXml(candidateSessionSummaryMetadata.otherOutcomeIdentifiers[index])}</td>
+            <td>${fn:escapeXml(candidateSessionSummaryData.otherOutcomeValues[index])}</td>
           </tr>
         </c:forEach>
       </c:if>

diff --git a/...ks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/showCandidateSessionEvents.jsp b/...ks-engine/src/main/webapp/WEB-INF/jsp/views/lti/instructor/showCandidateSessionEvents.jsp
@@ -15,7 +15,6 @@ candidateEventSummaryDataList
 <%@ include file="/WEB-INF/jsp/includes/pageheader.jspf" %>
 <c:set var="candidateSessionSummaryMetadata" value="${candidateSessionSummaryReport.candidateSessionSummaryMetadata}"/>
 <c:set var="candidateSessionSummaryData" value="${candidateSessionSummaryReport.candidateSessionSummaryData}"/>
-<c:set var="assessmentResultXml" value="${candidateSessionSummaryReport.assessmentResultXml}"/>
 <page:ltipage title="Candidate Session Activity Log">
 
   <header class="actionHeader">

diff --git a/qtiworks-engine/src/main/webapp/release-notes.jsp b/qtiworks-engine/src/main/webapp/release-notes.jsp
@@ -12,6 +12,26 @@ All Rights Reserved
   </nav>
   <h2>QTIWorks Release Notes</h2>
 
+  <h3>Release 1.0-beta7 (12/05/2014)</h3>
+  <p>
+    Security release. This fixes potential cross-site scripting (XSS) vulnerabilities
+    caused by a failure to escape user input in some instructor-facing JSP
+    pages, including the system user login page.
+  </p>
+  <p>
+    The <a href="https://webapps.ph.ed.ac.uk/qtiworks">public demo of QTIWorks</a> has been
+    upgraded in conjunction with this release.  All people running their own
+    QTIWorks installations are strongly encouraged to upgrade as soon as
+    possible.
+  </p>
+  <h4>Issues resolved</h4>
+  <ul>
+    <li>
+      <a href="https://github.com/davemckain/qtiworks/issues/45">#45</a>: XSS vulnerabilities
+      in some instructor interface pages.
+    </li>
+  </ul>
+
   <h3>Release 1.0-beta6 (15/04/2014)</h3>
   <p>
     This is hoped to be the final beta before a RC or final 1.0.0 release.