diff --git a/sonar-xml-plugin/src/main/java/org/sonar/plugins/xml/checks/security/android/AndroidPermissionsCheck.java b/sonar-xml-plugin/src/main/java/org/sonar/plugins/xml/checks/security/android/AndroidPermissionsCheck.java index 007e489b8..16772121a 100644 --- a/sonar-xml-plugin/src/main/java/org/sonar/plugins/xml/checks/security/android/AndroidPermissionsCheck.java +++ b/sonar-xml-plugin/src/main/java/org/sonar/plugins/xml/checks/security/android/AndroidPermissionsCheck.java @@ -18,20 +18,23 @@ import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Set; import javax.xml.xpath.XPathExpression; import org.sonar.check.Rule; import org.sonarsource.analyzer.commons.xml.XPathBuilder; import org.sonarsource.analyzer.commons.xml.XmlFile; +import org.w3c.dom.Node; +import static org.sonar.plugins.xml.checks.security.android.Utils.ANDROID_MANIFEST_TOOLS; import static org.sonar.plugins.xml.checks.security.android.Utils.ANDROID_MANIFEST_XMLNS; @Rule(key = "S5604") public class AndroidPermissionsCheck extends AbstractAndroidManifestCheck { private static final String MESSAGE = "Make sure the use of \"%s\" permission is necessary."; - private final XPathExpression xPathExpression = XPathBuilder.forExpression("/manifest/uses-permission/@n1:name") - .withNamespace("n1", ANDROID_MANIFEST_XMLNS) + private final XPathExpression xPathExpression = XPathBuilder + .forExpression("/manifest/uses-permission") .build(); private static final Set DANGEROUS_PERMISSIONS = new HashSet<>(Arrays.asList( @@ -110,13 +113,37 @@ public class AndroidPermissionsCheck extends AbstractAndroidManifestCheck { @Override protected final void scanAndroidManifest(XmlFile file) { - evaluateAsList(xPathExpression, file.getDocument()).stream() - .filter(node -> DANGEROUS_PERMISSIONS.contains(node.getNodeValue())) - .forEach(node -> reportIssue(node, String.format(MESSAGE, simpleName(node.getNodeValue())))); + List usesPermissionNodes = evaluateAsList(xPathExpression, file.getDocument()); + if (!hasToolsNodeRemoveAll(usesPermissionNodes)) { + usesPermissionNodes.forEach(this::checkAndReportPermissionIssue); + } + } + + private void checkAndReportPermissionIssue(Node node) { + Node permissionsAttribute = findPermissionAttribute(node); + String permissionValue = permissionsAttribute.getNodeValue(); + if (!hasToolsNodeValue(node, "remove") + && DANGEROUS_PERMISSIONS.contains(permissionValue)) { + reportIssue(permissionsAttribute, String.format(MESSAGE, simpleName(permissionValue))); + } } private static String simpleName(String fullyQualifiedName) { return fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf('.') + 1); } + private static Node findPermissionAttribute(Node node) { + return node.getAttributes().getNamedItemNS(ANDROID_MANIFEST_XMLNS, "name"); + } + + private static boolean hasToolsNodeValue(Node node, String value) { + Node toolsNodeAttribute = node.getAttributes().getNamedItemNS(ANDROID_MANIFEST_TOOLS, "node"); + return toolsNodeAttribute != null && value.equals(toolsNodeAttribute.getNodeValue()); + } + + private static boolean hasToolsNodeRemoveAll(List permissionNodes) { + return permissionNodes + .stream() + .anyMatch(node -> hasToolsNodeValue(node, "removeAll")); + } } diff --git a/sonar-xml-plugin/src/main/java/org/sonar/plugins/xml/checks/security/android/Utils.java b/sonar-xml-plugin/src/main/java/org/sonar/plugins/xml/checks/security/android/Utils.java index 1c97ad0a5..a8d7bfe6d 100644 --- a/sonar-xml-plugin/src/main/java/org/sonar/plugins/xml/checks/security/android/Utils.java +++ b/sonar-xml-plugin/src/main/java/org/sonar/plugins/xml/checks/security/android/Utils.java @@ -24,6 +24,7 @@ private Utils() { } public static final String ANDROID_MANIFEST_XMLNS = "http://schemas.android.com/apk/res/android"; + public static final String ANDROID_MANIFEST_TOOLS = "http://schemas.android.com/tools"; public static final String ANDROID_MANIFEST_FILENAME = "AndroidManifest.xml"; public static boolean isAndroidManifestFile(XmlFile file) { diff --git a/sonar-xml-plugin/src/test/java/org/sonar/plugins/xml/checks/security/android/AndroidPermissionsCheckTest.java b/sonar-xml-plugin/src/test/java/org/sonar/plugins/xml/checks/security/android/AndroidPermissionsCheckTest.java index 5e87677ba..79f29e32c 100644 --- a/sonar-xml-plugin/src/test/java/org/sonar/plugins/xml/checks/security/android/AndroidPermissionsCheckTest.java +++ b/sonar-xml-plugin/src/test/java/org/sonar/plugins/xml/checks/security/android/AndroidPermissionsCheckTest.java @@ -32,4 +32,18 @@ void not_manifest() { SonarXmlCheckVerifier.verifyNoIssue(Paths.get("AnyFileName.xml").toString(), new AndroidPermissionsCheck()); } + @Test + void should_not_raise_on_tools_node_remove() { + SonarXmlCheckVerifier.verifyIssues(Paths.get("ToolsNodeRemove/AndroidManifest.xml").toString(), new AndroidPermissionsCheck()); + } + + @Test + void should_not_raise_on_tools_node_remove_all() { + SonarXmlCheckVerifier.verifyNoIssue(Paths.get("ToolsNodeRemoveAll/AndroidManifest.xml").toString(), new AndroidPermissionsCheck()); + } + + @Test + void should_raise_on_tools_node_remove_all_with_wrong_tag() { + SonarXmlCheckVerifier.verifyIssues(Paths.get("ToolsNodeRemoveAllWrongTag/AndroidManifest.xml").toString(), new AndroidPermissionsCheck()); + } } diff --git a/sonar-xml-plugin/src/test/resources/checks/AndroidPermissionsCheck/ToolsNodeRemove/AndroidManifest.xml b/sonar-xml-plugin/src/test/resources/checks/AndroidPermissionsCheck/ToolsNodeRemove/AndroidManifest.xml new file mode 100644 index 000000000..ca5b083e9 --- /dev/null +++ b/sonar-xml-plugin/src/test/resources/checks/AndroidPermissionsCheck/ToolsNodeRemove/AndroidManifest.xml @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/sonar-xml-plugin/src/test/resources/checks/AndroidPermissionsCheck/ToolsNodeRemoveAll/AndroidManifest.xml b/sonar-xml-plugin/src/test/resources/checks/AndroidPermissionsCheck/ToolsNodeRemoveAll/AndroidManifest.xml new file mode 100644 index 000000000..54e377fd1 --- /dev/null +++ b/sonar-xml-plugin/src/test/resources/checks/AndroidPermissionsCheck/ToolsNodeRemoveAll/AndroidManifest.xml @@ -0,0 +1,11 @@ + + + + + + + + + + diff --git a/sonar-xml-plugin/src/test/resources/checks/AndroidPermissionsCheck/ToolsNodeRemoveAllWrongTag/AndroidManifest.xml b/sonar-xml-plugin/src/test/resources/checks/AndroidPermissionsCheck/ToolsNodeRemoveAllWrongTag/AndroidManifest.xml new file mode 100644 index 000000000..f3e14cfbf --- /dev/null +++ b/sonar-xml-plugin/src/test/resources/checks/AndroidPermissionsCheck/ToolsNodeRemoveAllWrongTag/AndroidManifest.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + +