CODEWITHSUNDEEP
javajspxss
schu777
schu777

Reputation: 366

CheckMarx - Cross-Site-Scripting attack

I'm a newbie to the CheckMarx and preventing XSS attacks. I've got this error:

Method %> at line 1 of \app\src\web\searchresults.jsp gets user input for the getSearchResults element. This element’s value then flows through the code without being properly sanitized or validated and is eventually displayed to the user in method %> at line 1 of \app\src\web\searchresults.jsp. This may enable a Cross-Site-Scripting attack.

This is the line that it complaining about in the code snip below:

<c:forEach var="combo" items="${searchForm.searchResults}">

Code snip from a searchresults.jsp (code modified to protect the innocent)

<% int i = 0;%>
<c:forEach var="combo" items="${searchForm.searchResults}">
  <tr <%=i++%2==0?"":"class='odd'"%>>
  <td align="center" style="width: 40px;"><c:out value="${combo.stay.status}"/></td>
  <c:choose>
    <c:when test="${hasDetailAccess}">
    <c:url var="detailLink" value="/detail.do">
    <c:param name="code" value="${searchForm.code}"/>
    <c:param name="brandCode" value="${searchForm.brandCode}"/>
    <c:param name="smUni" value="${combo.object1.smUni}"/>
    <c:param name="shUni" value="${combo.object1.shUni}"/>
    <c:param name="searchType" value="${searchForm.searchType}"/>
  </c:choose>
  <td style="width: 80px;"><fmt:formatDate pattern="MMM dd yyyy" value="${combo.object1.dateMade.date}"/></td>
  <td style="width: 80px;"><c:out value="${combo.object1.lastName}"/></td>
  <td style="width: 80px;"><c:out value="${combo.object1.firstName}"/></td>
</c:forEach>

The part that I'm not for sure of how to fix is that the "searchForm.searchResults" is an queueCombo that can refer to multiple objects, for this instance, "object1".

My thought is to do the clean-up on the object. My method of doing this would be to add the needed attributes the page is using (as the "object1" is HUGE number of attributes) and put as single values and populate them when the object is constructed.

Upvotes: 3

Views: 2779

Answers (1)

Fabien
Fabien

Reputation: 4972

I don't have a specific solution for sanitizing your "queueCombo" after that it has been built, but generally the best-practices for securing the user inputs are:

  • as soon as the value is received from the user (early control): here before building your object. Most sanitizations made early intend to remove forbidden characters and control values formats. Eg: here the first and last names should not contain anything else than alphabetical characters.

You should not perform a transformation of variables early if the transformation aims to encode the data in a specific format (eg HTML output) because it could made the use of the objects in other contexts (eg: DB queries) harder to do.

  • just before using the variable (late control). Eg: SQL sanitization prior making a SQL query, or HTML encoding prior printing the values.

I think, in regards of your code snippet, that the latter should do.

Checkmarx is alerting on the loop line, but the issue is not really there. It lies in the value= statements that follow, the sanitization should be made here.

Tip for other readers: do not forget that any sanitization process should be made server side. If made client-side, it is mostly cosmetical.

Upvotes: 1

Related Questions