Popular Vulnerable Code

Ki’s from overseas –XSS

Details

Affected Software:JSPWiki

Fixed in Version:2.60

Issue Type:Cross Site Scripting

Original Code:Found Here

Description

The vulnerable code from JSPWiki contained persistent Cross Site Scripting vulnerabilities.  Although we cannot see the call to session.setAttribute(),we can see that the author,link,and remember values were not sanitized before being used in FORM input fields.  The exposure was fixed by using “c:out”.  c:out is part of the JSP Standard Tag Library (JSTL) and is used to output to the current JspWriter.  If the “escapeXML” attribute is set to “true” (which is the default value),c:out will escape some HTML characters,helping to reduce the possibility of XSS.  By default,c:out encodes the following characters:<,>,&,“,and ‘.

This week’s code sample brings up the old application security argument as to whether it’s better to sanitize input as it arrives or to escape data at the point of consumption (or as near to the point of consumption as possible).  There are pros and cons for both methods.  In this case,the JSPWiki developers chose to encode at the point of consumption.  If the author,link,and remember values were instead sanitized when being assigned to the session store,the developers would have also prevented the XSS exposure… so which is better?

From a security code review perspective,encoding data at the point of entry requires additional flow analysis,as the code reviewer has to follow the data from the point of consumption back to assignment in order to ensure it hasn’t been tainted along the way before being consumed.  Sanitizing input as it arrives can be challenging as it may be difficult to determine how best to sanitize incoming data if that data is to be used in diverse ways throughout the application (inserted into a database,output to HTML,used in LDAP queries,used as part of a mail API…etc.).  This typically results in more generic forms of sanitization routines which may not stand up to targeted or sophisticated attacks.  Sanitizing data as it arrives does offer the advantage of being able to sanitize a particular piece of data once,as opposed to tracking down each instance of consumption and sanitizing at the points of consumption.

Sanitizing at the point of consumption offers advantages as the developer has a clear understanding as to how the data will be used (inserted into a database,output to HTML,used in LDAP queries,used as part of a mail API…etc.) and can implement the best sanitization routine for the situation.  From a security code review perspective,code that sanitizes at the point of consumption requires less flow analysis as each consumption point can be evaluated independently of other consumption points.  This independence comes at a cost as each point of consumption must now be evaluated in order to ensure the proper sanitization is being put in place.  A single missed consumption point will likely result in the introduction of a vulnerability in the application.

It’s also interesting seeing the vulnerable code sample has comments related to Edit.jsp and Comment.jsp.  It seems the conditionals for these two JSP files may not be very robust and could be an interesting avenue for attack.  I’m also curious as to whether this particular FORM has a robust CSRF token…

Developers Solution

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
<% if( usertext == null ) usertext = "";%>
<%
String action = "comment".equals(request.getParameter("action")) ?
context.getURL(WikiContext.COMMENT,context.getName()):
context.getURL(WikiContext.EDIT,context.getName());
%>
<form accept-charset="<wiki:ContentEncoding/>"method="post"
action="<%=action%>"
name="editForm"enctype="application/x-www-form-urlencoded">
<p>
<%-- Edit.jsp &Comment.jsp rely on these being found.  So be careful,if you make changes. --%>
-<input name="author"type="hidden"value="<%=session.getAttribute("author")%>"/>
-<input name="link"type="hidden"value="<%=session.getAttribute("link")%>"/>
-<input name="remember"type="hidden"value="<%=session.getAttribute("remember")%>"/>
+<input type="hidden"name="author"value="<c:out value='${author}' />"/>
+<input type="hidden"name="link"value="<c:out value='${link}' />"/>
+    <input type="hidden"name="remember"value="<c:out value='${remember}' />"/>

<input name="page"type="hidden"value="<wiki:Variable var="pagename"/>"/>
<input name="action"type="hidden"value="save"/>
-        <input name="edittime"type="hidden"value="<%=pageContext.getAttribute("lastchange",
PageContext.REQUEST_SCOPE )%>"/>
-        <input name="addr"type="hidden"value="<%=request.getRemoteAddr()%>"/>
+    <input name="<%=SpamFilter.getHashFieldName(request)%>"type="hidden"value="<c:out value='${lastchange}' />"/>
November 13th,2009 | Category:Cross-Site Scripting (XSS),Java,JSPWiki,Solution | Leave a comment

Ki’s From Overseas

I got ki’s,comin from overseas…
- Sykes

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
<% if( usertext == null ) usertext = ""; %>
<%
String action = "comment".equals(request.getParameter("action")) ?
context.getURL(WikiContext.COMMENT,context.getName()) :
context.getURL(WikiContext.EDIT,context.getName());
%>
<form accept-charset="<wiki:ContentEncoding/>" method="post"
action="<%=action%>"
name="editForm" enctype="application/x-www-form-urlencoded">
<p>
<%-- Edit.jsp & Comment.jsp rely on these being found.  So be careful,if you make changes. --%>
<input name="author" type="hidden" value="<%=session.getAttribute("author")%>" />
<input name="link" type="hidden" value="<%=session.getAttribute("link")%>" />
<input name="remember" type="hidden" value="<%=session.getAttribute("remember")%>" />

<input name="page" type="hidden" value="<wiki:Variable var="pagename"/>" />
<input name="action" type="hidden" value="save" />
<input name="edittime" type="hidden" value="<%=pageContext.getAttribute("lastchange",
PageContext.REQUEST_SCOPE )%>" />
<input name="addr" type="hidden" value="<%=request.getRemoteAddr()%>" />
November 9th,2009 | Category:Code Snippet,Java | One comment

Disorderly Discovery – XSS

Details

Affected Software:JSPWiki

Fixed in Version:2.5.164

Issue Type: Cross Site Scripting (XSS)

Original Code:Found Here

Description

This is a classic XSS vulnerability which affected JSPWiki.

The setId() method assigns a non-sanitized value to the m_tabID variable. The m_tabID variable is then later used in the doWikiStartTag() method as part of a stringbuffer which is used to build dynamic HTML.  The m_tabID variable is never encoded or sanitized before being echoed back to the user.

In addition,to the setId() method,the setTitle(),setAccessKey(),and setUrl() methods also expose a potential for XSS.  The JSPWiki team used TextUtil.replaceEntities() in each of the vulnerable methods to sanitize the values being assigned to the respective variables.

If you had to write the TextUtil.replaceEntities() method,what would it look like?

Developers Solution

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
public class TabTag extends WikiTagBase {
private static final long serialVersionUID = -8534125226484616489L;
private String m_accesskey;
private String m_tabID;
private String m_tabTitle;
private String m_url;

public void setId(String aTabID)
{
-   m_tabID = aTabID;
+   m_tabID = TextUtil.replaceEntities( aTabID );
}

public void setTitle(String aTabTitle)
{
-    m_tabTitle = aTabTitle;
+    m_tabTitle = TextUtil.replaceEntities( aTabTitle );
}

public void setAccesskey(String anAccesskey)
{
-    m_accesskey = anAccesskey;//take only the first char
+    m_accesskey = TextUtil.replaceEntities( anAccesskey );//take only the first char
}

public void setUrl( String url )
{
-    m_url = url;
+    m_url = TextUtil.replaceEntities( url );
}

public int doWikiStartTag() throws JspTagException {
TabbedSectionTag parent = (TabbedSectionTag) findAncestorWithClass(
this,TabbedSectionTag.class);

if (m_tabID == null) {
throw new JspTagException("Tab Tag without \"id\"attribute");
}
if (m_tabTitle == null) {
throw new JspTagException("Tab Tag without \"tabTitle\"attribute");
}
if (parent == null) {
throw new JspTagException(
"Tab Tag without parent \"TabbedSection\"Tag");
}

if (!parent.isStateGenerateTabBody())
return SKIP_BODY;

StringBuffer sb = new StringBuffer(32);

sb.append("<div id=\""+ m_tabID + "\"");

if (!parent.validateDefaultTab(m_tabID)) {
sb.append("class=\"hidetab\"");
}
sb.append(">\n");

try {
pageContext.getOut().write(sb.toString());
} catch (java.io.IOException e) {
throw new JspTagException("IO Error:"+ e.getMessage());
}

return EVAL_BODY_INCLUDE;
}

}
October 2nd,2009 | Category:Cross-Site Scripting (XSS),Java,Solution | Leave a comment

Disorderly Discovery

One of the advantages of being disorderly is that one is constantly making exciting discoveries.
- A. A. Milne

Vulnerable Code

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
public class TabTag extends WikiTagBase {
private static final long serialVersionUID = -8534125226484616489L;
private String m_accesskey;
private String m_tabID;
private String m_tabTitle;
private String m_url;

public void setId(String aTabID) {
m_tabID = aTabID;
}

public void setTitle(String aTabTitle) {
m_tabTitle = aTabTitle;
}

public void setAccesskey(String anAccesskey) {
m_accesskey = anAccesskey; // take only the first char
}

public void setUrl(String url) {
m_url = url;
}

public int doWikiStartTag() throws JspTagException {
TabbedSectionTag parent = (TabbedSectionTag) findAncestorWithClass(this,TabbedSectionTag.class);

if (m_tabID == null) {
throw new JspTagException("Tab Tag without \"id\" attribute");
}
if (m_tabTitle == null) {
throw new JspTagException("Tab Tag without \"tabTitle\" attribute");
}
if (parent == null) {
throw new JspTagException("Tab Tag without parent \"TabbedSection\" Tag");
}

if (!parent.isStateGenerateTabBody())
return SKIP_BODY;

StringBuffer sb = new StringBuffer(32);

sb.append("<div id=\"" + m_tabID + "\"");

if (!parent.validateDefaultTab(m_tabID)) {
sb.append("class=\"hidetab\"");
}
sb.append(">\n");

try {
pageContext.getOut().write(sb.toString());
} catch (java.io.IOException e) {
throw new JspTagException("IO Error:" + e.getMessage());
}

return EVAL_BODY_INCLUDE;
}
}
September 28th,2009 | Category:Code Snippet,Java | 2 comments
«Newer Entries