phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug fixes for incorrect detection of id's

Browse Source
This commit is contained in:
Sergey
2017-03-29 14:41:45 -07:00
parent 6ab58da6af
commit 16b7cbcb4e
2 changed files with 55 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
pmd-visualforce/src/main/java/net/sourceforge/pmd/lang/vf/rule/security/VfUnescapeElRule.java
View File

@ -83,7 +83,8 @@ public class VfUnescapeElRule extends AbstractVfRule {
if (quoted) {
// check escaping too
if (!(startsWithSafeResource(elExpression) || containsSafeFields(elExpression))) {
if (doesElContainAnyUnescapedIdentifiers(elExpression, Escaping.JSENCODE)) {
if (doesElContainAnyUnescapedIdentifiers(elExpression,
EnumSet.of(Escaping.JSENCODE, Escaping.JSINHTMLENCODE))) {
addViolation(data, elExpression);
}
}
@ -343,7 +344,7 @@ public class VfUnescapeElRule extends AbstractVfRule {
final List<ASTExpression> exprs = elExpression.findChildrenOfType(ASTExpression.class);
for (final ASTExpression expr : exprs) {
if (containsSafeFields(expr)) {
if (innerContainsSafeFields(expr)) {
continue;
}
@ -381,7 +382,13 @@ public class VfUnescapeElRule extends AbstractVfRule {
}
private boolean containsSafeFields(final AbstractVFNode expression) {
final ASTExpression ex = expression.getFirstChildOfType(ASTExpression.class);
return ex == null ? false : innerContainsSafeFields(ex);
}
private boolean innerContainsSafeFields(final AbstractVFNode expression) {
for (int i = 0; i < expression.jjtGetNumChildren(); i++) {
Node child = expression.jjtGetChild(i);
@ -396,7 +403,7 @@ public class VfUnescapeElRule extends AbstractVfRule {
}
if (child instanceof ASTDotExpression) {
if (containsSafeFields((ASTDotExpression) child)) {
if (innerContainsSafeFields((ASTDotExpression) child)) {
return true;
}
}

58
pmd-visualforce/src/test/resources/net/sourceforge/pmd/lang/vf/rule/security/xml/VfUnescapeEl.xml
View File

@ -93,6 +93,51 @@ Safe case id in script
</test-code>
<test-code>
<description><![CDATA[
Safely escaped case in script context
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
<apex:page>
<script>
window.parent.opener.location.href = '{!JSINHTMLENCODE(case)}';
</script>
</apex:page>
]]></code>
<source-type>vf</source-type>
</test-code>
<test-code>
<description><![CDATA[
Id in the EL means no XSS
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
<apex:page>
<a onclick="ShowUnregisterWindow('{!item.id}')">foo</a>
</apex:page>
]]></code>
<source-type>vf</source-type>
</test-code>
<test-code>
<description><![CDATA[
User Id is safe
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
<apex:page>
<script>
$Lightning.createComponent("c:RE_TasksListComp",{"loginUserId":"{!$User.Id}"});
</script>
</apex:page>
]]></code>
<source-type>vf</source-type>
</test-code>
<test-code>
<description><![CDATA[
No XSS in safe script commands quoted context
@ -194,19 +239,6 @@ Starts with Resource
<source-type>vf</source-type>
</test-code>
<test-code>
<description><![CDATA[
Id in the EL means no XSS
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
<apex:page>
<a onclick="ShowUnregisterWindow('{!item.id}')">foo</a>
</apex:page>
]]></code>
<source-type>vf</source-type>
</test-code>
<test-code>
<description><![CDATA[
EL in JS on-event handler - stored XSS