From ee4763e2fd1c224d8a8a8218ca2091509712d366 Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Wed, 18 Dec 2024 11:24:24 -0500 Subject: [PATCH] NEW(pmd): @W-17310939@: Add in more AppExchange rules for xml language --- .../pmd-rules/build.gradle.kts | 11 + .../pmd/xml/DetectSecretsInCustomObjects.java | 195 ++++++++++++++++++ .../sfca/rulesets/AppExchange_xml.xml | 183 +++++++++++++++- .../AvoidApiSessionIdTest.java | 16 ++ .../AvoidAuraWithLockerDisabledTest.java | 16 ++ .../AvoidDisableProtocolSecurityTest.java | 16 ++ .../AvoidJavaScriptCustomObjectTest.java | 16 ++ .../AvoidJavaScriptHomePageComponentTest.java | 16 ++ .../AvoidLmcIsExposedTrueTest.java | 16 ++ .../appexchange_xml/AvoidSControlsTest.java | 16 ++ .../LimitConnectedAppScopeTest.java | 16 ++ .../ProtectSensitiveDataTest.java | 11 + .../UseHttpsCallbackUrlTest.java | 16 ++ .../appexchange_xml/xml/AvoidApiSessionId.xml | 58 ++++++ .../xml/AvoidAuraWithLockerDisabled.xml | 32 +++ .../xml/AvoidDisableProtocolSecurity.xml | 32 +++ .../AvoidInsecureHttpRemoteSiteSetting.xml | 14 +- .../xml/AvoidJavaScriptCustomObject.xml | 34 +++ .../xml/AvoidJavaScriptHomePageComponent.xml | 49 +++++ .../xml/AvoidLmcIsExposedTrue.xml | 32 +++ .../appexchange_xml/xml/AvoidSControls.xml | 19 ++ .../xml/LimitConnectedAppScope.xml | 43 ++++ .../xml/UseHttpsCallbackUrl.xml | 58 ++++++ .../code-analyzer-pmd-engine/src/constants.ts | 11 +- .../src/pmd-rule-mappings.ts | 50 +++++ .../rules_allLanguages.goldfile.json | 112 +++++++++- 26 files changed, 1062 insertions(+), 26 deletions(-) create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/main/java/com/salesforce/security/pmd/xml/DetectSecretsInCustomObjects.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidApiSessionIdTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidAuraWithLockerDisabledTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidDisableProtocolSecurityTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidJavaScriptCustomObjectTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidJavaScriptHomePageComponentTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidLmcIsExposedTrueTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidSControlsTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/LimitConnectedAppScopeTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/ProtectSensitiveDataTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/UseHttpsCallbackUrlTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidApiSessionId.xml create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidAuraWithLockerDisabled.xml create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidDisableProtocolSecurity.xml create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidJavaScriptCustomObject.xml create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidJavaScriptHomePageComponent.xml create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidLmcIsExposedTrue.xml create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidSControls.xml create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/LimitConnectedAppScope.xml create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/UseHttpsCallbackUrl.xml diff --git a/packages/code-analyzer-pmd-engine/pmd-rules/build.gradle.kts b/packages/code-analyzer-pmd-engine/pmd-rules/build.gradle.kts index 5c323418..d00b0b8a 100644 --- a/packages/code-analyzer-pmd-engine/pmd-rules/build.gradle.kts +++ b/packages/code-analyzer-pmd-engine/pmd-rules/build.gradle.kts @@ -74,6 +74,17 @@ tasks.test { // The jacocoTestReport and jacocoTestCoverageVerification tasks must be run separately from the test task // Otherwise, running a single test from the IDE will trigger this verification. tasks.jacocoTestCoverageVerification { + + // Exclude specific classes from the coverage verification calculation + classDirectories.setFrom( + fileTree("build/classes/java/main").filter { + // Normalize the path to use forward slashes for comparison purposes + val normalizedPath = it.path.replace("\\", "/") + // Exclude the DetectSecretsInCustomObjects class because the Security team hasn't given us a valid test for it yet: + !normalizedPath.contains("com/salesforce/security/pmd/xml/DetectSecretsInCustomObjects.class") + } + ) + violationRules { rule { limit { diff --git a/packages/code-analyzer-pmd-engine/pmd-rules/src/main/java/com/salesforce/security/pmd/xml/DetectSecretsInCustomObjects.java b/packages/code-analyzer-pmd-engine/pmd-rules/src/main/java/com/salesforce/security/pmd/xml/DetectSecretsInCustomObjects.java new file mode 100644 index 00000000..a6f277ce --- /dev/null +++ b/packages/code-analyzer-pmd-engine/pmd-rules/src/main/java/com/salesforce/security/pmd/xml/DetectSecretsInCustomObjects.java @@ -0,0 +1,195 @@ +package com.salesforce.security.pmd.xml; + +import net.sourceforge.pmd.lang.xml.ast.internal.XmlParserImpl.RootXmlNode; +import java.io.File; +import java.io.IOException; +import java.util.List; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathFactory; + +import org.w3c.dom.Document; +import org.w3c.dom.NodeList; + +import net.sourceforge.pmd.reporting.RuleContext; +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.document.FileId; +import net.sourceforge.pmd.lang.rule.AbstractRule; +import org.xml.sax.SAXException; + +public class DetectSecretsInCustomObjects extends AbstractRule { + private static final List PRIVACY_FIELD_MAPPINGS_LIST = List.of( + "SSN", + "SOCIALSECURITY", + "SOCIAL_SECURITY", + "NATIONALID", + "NATIONAL_ID", + "NATIONAL_IDENTIFIER", + "NATIONALIDENTIFIER", + "DRIVERSLICENSE", + "DRIVERS_LICENSE", + "DRIVER_LICENSE", + "DRIVERLICENSE", + "PASSPORT", + "AADHAAR", + "AADHAR" //More? + ); + + private static final List AUTH_FIELD_MAPPINGS_LIST = List.of( + "KEY", // potentially high false +ve rate + "ACCESS", + "PASS", + "ENCRYPT", + "TOKEN", + "HASH", + "SECRET", + "SIGNATURE", + "SIGN", + "AUTH", //AUTHORIZATION,AUTHENTICATION,AUTHENTICATE,OAUTH + "AUTHORIZATION", + "AUTHENTICATION", + "AUTHENTICATE", + "BEARER", + "CRED", //cred, credential(s), + "REFRESH", // + "CERT", + "PRIVATE", + "PUBLIC", + "JWT" + ); + + public static final String VISIBILITY_XPATH_EXPR = "/CustomObject/visibility[text()=\"Public\"]"; + public static final String PRIVACY_FIELD_XPATH_EXPR = "/CustomField/type[text()!=\"EncryptedText\"]"; + private static final String CUSTOM_SETTINGS_XPATH_EXPR = "/CustomObject/customSettingsType"; + + @Override + public void apply(Node target, RuleContext ctx) { + FileId fId = target.getReportLocation().getFileId(); + String fieldFileName = fId.getAbsolutePath(); + if (!fieldFileName.endsWith(".field-meta.xml")) { + return; + } + String fieldName = fieldNameFromFileName(fieldFileName); + if (isAnAuthTokenField(fieldName)) { + doObjectVisibilityCheck(ctx, target, fieldFileName); + } + if (isAnInsecurePrivacyField(fieldName)) { + checkFieldType(ctx, target, fieldName); + } + } + + private void checkFieldType(RuleContext ctx, Node node, String fieldName) { + RootXmlNode pmdRootNode = (RootXmlNode) node; + Document xmlDoc = pmdRootNode.getNode(); + if (isXPathExpFoundInDocument(xmlDoc, PRIVACY_FIELD_XPATH_EXPR)) { //not an EncryptedText type + ctx.addViolationWithMessage(pmdRootNode, + fieldName +" is a potential privacy field and is not an EncryptedText"); + } + } + + private void doObjectVisibilityCheck(RuleContext ctx, Node pmdRootNode, String fieldFileName) { + String fieldName = fieldNameFromFileName(fieldFileName); + + String objectFileName = getObjectFileName(fieldFileName); + if (objectFileName == null) { + return; + } + String objectName = objectNameFromFileName(objectFileName); + + if (objectName.endsWith("__mdt") || isCustomSettingsObject(objectFileName)) { + if (isObjectVisibilityPublic(objectFileName)) { + ctx.addViolationWithMessage(pmdRootNode, fieldName + + " is a potential auth token in the object: " + + objectName + " with public visibility"); + } + } else if (objectName.endsWith("__c")) { + ctx.addViolationWithMessage(pmdRootNode, fieldName + + " is a potential auth token in a custom object: " + + objectName); + } + } + + private boolean isCustomSettingsObject(String filename) { + return isXPathExpFoundInDocument(filename, CUSTOM_SETTINGS_XPATH_EXPR); + } + + private boolean isObjectVisibilityPublic(String filename) { + return isXPathExpFoundInDocument(filename, VISIBILITY_XPATH_EXPR); + } + + private boolean isXPathExpFoundInDocument(String filename, String customSettingsXpathExpr) { + try { + return isXPathExpFoundInDocument(parseDocument(filename), customSettingsXpathExpr); + } catch (Exception e) { + return false; //TBD: Handle the exception properly + } + } + + private boolean isXPathExpFoundInDocument(Document parsedXml, String customSettingsXpathExpr) { + try { + XPath xPath = XPathFactory.newInstance().newXPath(); + NodeList nodeList = (NodeList)xPath + .compile(customSettingsXpathExpr) + .evaluate(parsedXml, XPathConstants.NODESET); + + return nodeList.getLength() > 0; + } + catch (Exception e) { + return false; //TBD: Handle the exception properly + } + } + + private String getObjectFileName(String fieldFileName) { + File fieldFile = new File(fieldFileName); + try { + File objectDirFile = fieldFile.getParentFile() //fields directory + .getParentFile(); //must be the objectName + String objectNamePath = objectDirFile.getAbsolutePath(); + String objectName = objectDirFile.getName(); + String objectDefinitionFileName = objectName + ".object-meta.xml"; + return objectNamePath + File.separator + objectDefinitionFileName; + } + catch(Exception e) { + return null; //TBD: Handle the exception properly + } + } + + public boolean isAnAuthTokenField(String fieldName) { + return isAPartialMatchInList(fieldName.toUpperCase(), AUTH_FIELD_MAPPINGS_LIST); + } + + public boolean isAnInsecurePrivacyField(String fieldName) { + return isAPartialMatchInList(fieldName.toUpperCase(), PRIVACY_FIELD_MAPPINGS_LIST); + } + + + private static boolean isAPartialMatchInList(String inputStr, List listOfStrings) { + String inputStrUpper = inputStr.toUpperCase(); + for (String eachStr : listOfStrings) { + if (inputStrUpper.contains(eachStr)) { + return true; + } + } + return false; + } + + private static Document parseDocument(String xmlFile) throws ParserConfigurationException, SAXException, IOException { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setExpandEntityReferences(false); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + DocumentBuilder builder = factory.newDocumentBuilder(); + return builder.parse(new File(xmlFile)); + } + + private static String fieldNameFromFileName(String fieldFileName) { + return (new File(fieldFileName)).getName().replaceFirst(".field-meta.xml",""); + } + + private static String objectNameFromFileName(String objectFileName) { + return (new File(objectFileName)).getName().replaceFirst(".object-meta.xml", ""); + } +} \ No newline at end of file diff --git a/packages/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml b/packages/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml index 9dbca504..40a86884 100644 --- a/packages/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml +++ b/packages/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml @@ -5,23 +5,194 @@ xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd"> AppExchange Security Rules + + + + Detects use of Api.Session_ID or GETSESSIONID() to retrieve a session ID. + 2 + + + + + + + + + + + Detects use of API versions with Lightning Locker disabled in Aura components. Use API version 40 or greater. + 1 + + + + + + + + + + + Detects if "Disable Protocol Security" setting is true. + 3 + + + + + + + + - - Detects instances of a Remote Site Settings that use HTTP.Use HTTPS instead. + + Detects instances of a Remote Site Settings that use HTTP. Use HTTPS instead. 3 - - - + ]]> + + + + + + + + Detects use of custom JavaScript actions in custom rules. + 2 + + + + + Detects use of custom JavaScript actions in home page components. + + 2 + + + ')) + or + contains(upper-case(@Text),upper-case('