forked from phoedos/pmd
Context aware escaping
This commit is contained in:
Sergey
committed by
Juan Martín Sotuyo Dodero
Juan Martín Sotuyo Dodero
parent
18d9c12467
commit
93f82fbd20
@ -4,6 +4,7 @@
|
||||
|
||||
package net.sourceforge.pmd.lang.vf.rule.security;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
@ -20,15 +21,11 @@ import net.sourceforge.pmd.lang.vf.rule.AbstractVfRule;
|
||||
*
|
||||
*/
|
||||
public class VfUnescapeElRule extends AbstractVfRule {
|
||||
|
||||
private static final String JSINHTMLENCODE = "JSINHTMLENCODE";
|
||||
private static final String URLENCODE = "URLENCODE";
|
||||
private static final String HTMLENCODE = "HTMLENCODE";
|
||||
private static final String APEX_PARAM = "apex:param";
|
||||
private static final String VALUE = "value";
|
||||
private static final String ITEM_VALUE = "itemValue";
|
||||
private static final String ITEM_VALUE = "itemvalue";
|
||||
private static final String ESCAPE = "escape";
|
||||
private static final String ITEM_ESCAPED = "itemEscaped";
|
||||
private static final String ITEM_ESCAPED = "itemescaped";
|
||||
private static final String APEX_OUTPUT_TEXT = "apex:outputtext";
|
||||
private static final String APEX_PAGE_MESSAGE = "apex:pagemessage";
|
||||
private static final String APEX_PAGE_MESSAGES = "apex:pagemessages";
|
||||
@ -38,62 +35,106 @@ public class VfUnescapeElRule extends AbstractVfRule {
|
||||
@Override
|
||||
public Object visit(ASTElement node, Object data) {
|
||||
if (doesTagSupportEscaping(node)) {
|
||||
final List<ASTAttribute> attributes = node.findChildrenOfType(ASTAttribute.class);
|
||||
boolean isUnescaped = false;
|
||||
boolean isEL = false;
|
||||
boolean hasPlaceholders = false;
|
||||
|
||||
for (ASTAttribute attr : attributes) {
|
||||
if (attr != null) {
|
||||
String name = attr.getName();
|
||||
switch (name) {
|
||||
case ESCAPE:
|
||||
case ITEM_ESCAPED:
|
||||
final ASTText text = attr.getFirstDescendantOfType(ASTText.class);
|
||||
if (text != null) {
|
||||
if (text.getImage().equalsIgnoreCase(FALSE)) {
|
||||
isUnescaped = true;
|
||||
}
|
||||
}
|
||||
break;
|
||||
case VALUE:
|
||||
case ITEM_VALUE:
|
||||
|
||||
final ASTElExpression elInVal = attr.getFirstDescendantOfType(ASTElExpression.class);
|
||||
if (doesElContainAnyUnescapedIdentifiers(elInVal)) {
|
||||
isEL = true;
|
||||
}
|
||||
|
||||
final ASTText textValue = attr.getFirstDescendantOfType(ASTText.class);
|
||||
if (textValue != null) {
|
||||
if (Pattern.compile("\\{(\\w|,|\\.|'|:|\\s)*\\}").matcher(textValue.getImage()).matches()) {
|
||||
hasPlaceholders = true;
|
||||
}
|
||||
}
|
||||
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
if (hasPlaceholders && isUnescaped) {
|
||||
if (hasELInInnerElements(node)) {
|
||||
addViolation(data, node);
|
||||
}
|
||||
}
|
||||
|
||||
if (isEL && isUnescaped) {
|
||||
addViolation(data, node);
|
||||
}
|
||||
checkTagsThatSupportEscaping(node, data);
|
||||
} else {
|
||||
checkAllOtherTags(node, data);
|
||||
}
|
||||
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
private boolean doesElContainAnyUnescapedIdentifiers(final ASTElExpression elExpression) {
|
||||
private void checkAllOtherTags(ASTElement node, Object data) {
|
||||
final List<ASTAttribute> attributes = node.findChildrenOfType(ASTAttribute.class);
|
||||
boolean isEL = false;
|
||||
|
||||
for (ASTAttribute attr : attributes) {
|
||||
String name = attr.getName().toLowerCase();
|
||||
// look for onevents
|
||||
if (Pattern.compile("^on(\\w)+$").matcher(name).matches()) {
|
||||
final List<ASTElExpression> elsInVal = attr.findDescendantsOfType(ASTElExpression.class);
|
||||
for (ASTElExpression el : elsInVal) {
|
||||
if (doesElContainAnyUnescapedIdentifiers(el,
|
||||
Arrays.asList(ESCAPING.JSINHTMLENCODE, ESCAPING.JSENCODE))) {
|
||||
isEL = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (name.equalsIgnoreCase("href") || name.equalsIgnoreCase("src")) {
|
||||
final List<ASTElExpression> elsInVal = attr.findDescendantsOfType(ASTElExpression.class);
|
||||
for (ASTElExpression el : elsInVal) {
|
||||
if (doesElContainAnyUnescapedIdentifiers(el, ESCAPING.URLENCODE)) {
|
||||
isEL = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (isEL) {
|
||||
addViolation(data, node);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
private void checkTagsThatSupportEscaping(ASTElement node, Object data) {
|
||||
final List<ASTAttribute> attributes = node.findChildrenOfType(ASTAttribute.class);
|
||||
boolean isUnescaped = false;
|
||||
boolean isEL = false;
|
||||
boolean hasPlaceholders = false;
|
||||
|
||||
for (ASTAttribute attr : attributes) {
|
||||
String name = attr.getName().toLowerCase();
|
||||
switch (name) {
|
||||
case ESCAPE:
|
||||
case ITEM_ESCAPED:
|
||||
final ASTText text = attr.getFirstDescendantOfType(ASTText.class);
|
||||
if (text != null) {
|
||||
if (text.getImage().equalsIgnoreCase(FALSE)) {
|
||||
isUnescaped = true;
|
||||
}
|
||||
}
|
||||
break;
|
||||
case VALUE:
|
||||
case ITEM_VALUE:
|
||||
|
||||
final List<ASTElExpression> elsInVal = attr.findDescendantsOfType(ASTElExpression.class);
|
||||
for (ASTElExpression el : elsInVal) {
|
||||
if (doesElContainAnyUnescapedIdentifiers(el, ESCAPING.HTMLENCODE)) {
|
||||
isEL = true;
|
||||
}
|
||||
}
|
||||
|
||||
final ASTText textValue = attr.getFirstDescendantOfType(ASTText.class);
|
||||
if (textValue != null) {
|
||||
if (Pattern.compile("\\{(\\w|,|\\.|'|:|\\s)*\\}").matcher(textValue.getImage()).matches()) {
|
||||
hasPlaceholders = true;
|
||||
}
|
||||
}
|
||||
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
if (hasPlaceholders && isUnescaped) {
|
||||
if (hasELInInnerElements(node)) {
|
||||
addViolation(data, node);
|
||||
}
|
||||
}
|
||||
|
||||
if (isEL && isUnescaped) {
|
||||
addViolation(data, node);
|
||||
}
|
||||
}
|
||||
|
||||
private boolean doesElContainAnyUnescapedIdentifiers(final ASTElExpression elExpression, ESCAPING escape) {
|
||||
return doesElContainAnyUnescapedIdentifiers(elExpression, Arrays.asList(escape));
|
||||
|
||||
}
|
||||
|
||||
private boolean doesElContainAnyUnescapedIdentifiers(final ASTElExpression elExpression, List<ESCAPING> escapes) {
|
||||
if (elExpression == null) {
|
||||
return false;
|
||||
}
|
||||
@ -104,15 +145,23 @@ public class VfUnescapeElRule extends AbstractVfRule {
|
||||
|
||||
for (final ASTIdentifier id : ids) {
|
||||
if (id.getImage() != null) {
|
||||
switch (id.getImage()) {
|
||||
case HTMLENCODE:
|
||||
case URLENCODE:
|
||||
case JSINHTMLENCODE:
|
||||
isEscaped = true;
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
for (ESCAPING e : escapes) {
|
||||
if (id.getImage().equalsIgnoreCase(e.toString())) {
|
||||
isEscaped = true;
|
||||
break;
|
||||
}
|
||||
|
||||
if (e.equals(ESCAPING.ANY)) {
|
||||
for (ESCAPING esc : ESCAPING.values()) {
|
||||
if (id.getImage().equalsIgnoreCase(esc.toString())) {
|
||||
isEscaped = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
@ -124,7 +173,7 @@ public class VfUnescapeElRule extends AbstractVfRule {
|
||||
return false;
|
||||
}
|
||||
|
||||
switch (node.getName().toLowerCase()) { // vf is case insensitive?
|
||||
switch (node.getName().toLowerCase()) { // vf is case insensitive
|
||||
case APEX_OUTPUT_TEXT:
|
||||
case APEX_PAGE_MESSAGE:
|
||||
case APEX_PAGE_MESSAGES:
|
||||
@ -146,7 +195,7 @@ public class VfUnescapeElRule extends AbstractVfRule {
|
||||
for (ASTAttribute attrib : innerAttributes) {
|
||||
final List<ASTElExpression> elsInVal = attrib.findDescendantsOfType(ASTElExpression.class);
|
||||
for (final ASTElExpression el : elsInVal) {
|
||||
if (doesElContainAnyUnescapedIdentifiers(el)) {
|
||||
if (doesElContainAnyUnescapedIdentifiers(el, ESCAPING.ANY)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@ -159,4 +208,23 @@ public class VfUnescapeElRule extends AbstractVfRule {
|
||||
return false;
|
||||
}
|
||||
|
||||
enum ESCAPING {
|
||||
HTMLENCODE("HTMLENCODE"),
|
||||
URLENCODE("URLENCODE"),
|
||||
JSINHTMLENCODE("JSINHTMLENCODE"),
|
||||
JSENCODE("JSENCODE"),
|
||||
ANY("ANY");
|
||||
|
||||
private final String text;
|
||||
|
||||
private ESCAPING(final String text) {
|
||||
this.text = text;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return text;
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@ -1,5 +1,75 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<test-data>
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
EL in JS on-event handler - stored XSS
|
||||
]]></description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
<apex:page>
|
||||
<div onclick="{!XSSHere(bah)}" />
|
||||
</apex:page>
|
||||
]]></code>
|
||||
<source-type>vf</source-type>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
EL in JS on-event handler - stored XSS
|
||||
]]></description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
<apex:page>
|
||||
<div onclick="{!JSENCODE(NoXSSHere)}" />
|
||||
<div onclick="{!JSINHTMLENCODE(NoXSSHere)}" />
|
||||
<div onclick="{!JSENCODE(NoXSSHere)}" />
|
||||
</apex:page>
|
||||
]]></code>
|
||||
<source-type>vf</source-type>
|
||||
</test-code>
|
||||
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
EL in JS src handler - stored XSS
|
||||
]]></description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
<apex:page>
|
||||
<iframe src="{!XSSHere}" />
|
||||
</apex:page>
|
||||
]]></code>
|
||||
<source-type>vf</source-type>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
EL in JS src handler - stored XSS
|
||||
]]></description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
<apex:page>
|
||||
<iframe src="{!URLENCODE(XSSHere)}" />
|
||||
</apex:page>
|
||||
]]></code>
|
||||
<source-type>vf</source-type>
|
||||
</test-code>
|
||||
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
EL in JS src handler - stored XSS
|
||||
]]></description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
<apex:page>
|
||||
<iframe src="{!XSSHere}" />
|
||||
</apex:page>
|
||||
]]></code>
|
||||
<source-type>vf</source-type>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
Default escaped EL - no XSS
|
||||
@ -15,7 +85,7 @@ Default escaped EL - no XSS
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
No XSS
|
||||
EL that is properly escaped should be no XSS
|
||||
]]></description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
|
||||
Reference in New Issue
Block a user