diff --git a/apex/lang/best-practice/ncino/accessModifiers/GlobalAccessModifiers.cls b/apex/lang/best-practice/ncino/accessModifiers/GlobalAccessModifiers.cls new file mode 100644 index 0000000000..e7ae63173f --- /dev/null +++ b/apex/lang/best-practice/ncino/accessModifiers/GlobalAccessModifiers.cls @@ -0,0 +1,49 @@ +/* + * global javadoc + * javadoc with global in it + * javadoc global + */ +// ruleid: global-access-modifiers +global without sharing class GlobalAccessModifiers { + // ruleid: global-access-modifiers + global String globalInstanceVariable = 'globalInstanceVariable'; + + // ok: global-access-modifiers + public String publicInstanceVariable = 'publicInstanceVariable'; + + // ok: global-access-modifiers + private String privateInstanceVariable = 'privateInstanceVariable'; + + // ok: global-access-modifiers + // This is a test comment that has the word global in it + // ruleid: global-access-modifiers + global static void myGlobalMethod() { } + + // ok: global-access-modifiers + public static void myPublicMethod() { } + + // ok: global-access-modifiers + private static void myPrivateMethod() { } + + // ok: global-access-modifiers + // This is another test comment with global + // ruleid: global-access-modifiers + global with sharing class TestGlobalClass { } + + // ok: global-access-modifiers + public with sharing class TestPublicClass { } + + // ok: global-access-modifiers + private without sharing class SystemMode { } + + // ok: global-access-modifiers + // Global test comment - last one + // ruleid: global-access-modifiers + global static String globalStaticVariable = 'globalStaticVariable'; + + // ok: global-access-modifiers + public static String publicStaticVariable = 'publicStaticVariable'; + + // ok: global-access-modifiers + private static String privateStaticVariable = 'privateStaticVariable'; +} diff --git a/apex/lang/best-practice/ncino/accessModifiers/GlobalAccessModifiers.yaml b/apex/lang/best-practice/ncino/accessModifiers/GlobalAccessModifiers.yaml new file mode 100644 index 0000000000..a08fae9c43 --- /dev/null +++ b/apex/lang/best-practice/ncino/accessModifiers/GlobalAccessModifiers.yaml @@ -0,0 +1,25 @@ +rules: + - id: global-access-modifiers + min-version: 1.44.0 + severity: WARNING + languages: + - apex + metadata: + cwe: + - 'CWE-284: Improper Access Control' + category: best-practice + technology: + - salesforce + references: + - https://cwe.mitre.org/data/definitions/284.html + message: >- + Global classes, methods, and variables should be avoided (especially in managed packages) as they can + never be deleted or changed in signature. Always check twice if something needs to be global. + patterns: + - pattern-regex: global [A-Za-z0-9_]{3,} + - pattern-not-regex: //(\s+([a-zA-Z]+\s+)+)[a-zA-Z]+ + - pattern-not-regex: '[*](\s+([a-zA-Z]+\s+)+)[a-zA-Z]+' + paths: + exclude: + - "*Test*" + - "*test*" diff --git a/apex/lang/best-practice/ncino/tests/UseAssertClass.cls b/apex/lang/best-practice/ncino/tests/UseAssertClass.cls new file mode 100644 index 0000000000..11cb3ea3a7 --- /dev/null +++ b/apex/lang/best-practice/ncino/tests/UseAssertClass.cls @@ -0,0 +1,13 @@ +public class UseAssertClass { + // ruleid: use-assert-class + System.assert(true); + + // ruleid: use-assert-class + System.assertEquals(1,1); + + // ruleid: use-assert-class + System.assertNotEquals(1,1); + + // ok: use-assert-class + Assert.areEqual(1,1); +} diff --git a/apex/lang/best-practice/ncino/tests/UseAssertClass.yaml b/apex/lang/best-practice/ncino/tests/UseAssertClass.yaml new file mode 100644 index 0000000000..3df72ec2f3 --- /dev/null +++ b/apex/lang/best-practice/ncino/tests/UseAssertClass.yaml @@ -0,0 +1,18 @@ +rules: + - id: use-assert-class + min-version: 1.44.0 + severity: WARNING + languages: + - generic + metadata: + category: best-practice + technology: + - salesforce + message: >- + Assert methods in the System class have been replaced with the Assert class: + https://developer.salesforce.com/docs/atlas.en-us.apexref.meta/apexref/apex_class_System_Assert.htm + pattern-regex: System\.assert + paths: + include: + - "*.cls" + - "UseAssertClass.cls" diff --git a/apex/lang/best-practice/ncino/urls/AbsoluteUrls.cls b/apex/lang/best-practice/ncino/urls/AbsoluteUrls.cls new file mode 100644 index 0000000000..9341825b2b --- /dev/null +++ b/apex/lang/best-practice/ncino/urls/AbsoluteUrls.cls @@ -0,0 +1,51 @@ +/* + * Test Controller with Absolute URLs + */ +public with sharing class AbsoluteUrls { + + public PageReference absoluteSalesforceUrlExample() { + // ruleid: absolute-urls + String strUrl = 'https://na8.salesforce.com/TestVFPage?AccountId=999'; + PageReference newUrl = new PageReference(strUrl); + newURL.setRedirect(true); + + return newURL; + } + + public PageReference absoluteNonSalesforceUrlExample() { + // ok: absolute-urls + String strUrl = 'https://www.website.com'; + PageReference newUrl = new PageReference(strUrl); + newURL.setRedirect(true); + + return newURL; + } + + public PageReference nonAbsoluteSalesforceUrlExample() { + // ok: absolute-urls + String strUrl = URL.getSalesforceBaseUrl().toExternalForm() + '/TestVFPage?AccountId=999'; + PageReference newUrl = new PageReference(strUrl); + newURL.setRedirect(true); + + return newURL; + } + + // Absolute Salesforce URL comment example + // ruleid: absolute-urls + // https://na8.salesforce.com/TestVFPage?AccountId=999 + + // Absolute non-Salesforce URL comment example + // ok: absolute-urls + // https://www.website.com + + // Non-absolute Salesforce URL comment example + // ok: absolute-urls + // URL.getSalesforceBaseUrl().toExternalForm() + '/TestVFPage?AccountId=999 + + // ruleid: absolute-urls + private static final String ABSOLUTE_SF_URL_CONSTANT = 'https://na8.salesforce.com/TestVFPage?AccountId=999'; + // ok: absolute-urls + private static final String ABSOLUTE_NON_SF_URL_CONSTANT = 'https://www.website.com'; + // ok: absolute-urls + private static final String NON_ASBOLUTE_SF_URL_CONSTANT = URL.getOrgDomainUrl().toExternalForm() + '/TestVFPage?AccountId=999'; +} diff --git a/apex/lang/best-practice/ncino/urls/AbsoluteUrls.yaml b/apex/lang/best-practice/ncino/urls/AbsoluteUrls.yaml new file mode 100644 index 0000000000..90f7057feb --- /dev/null +++ b/apex/lang/best-practice/ncino/urls/AbsoluteUrls.yaml @@ -0,0 +1,21 @@ +rules: + - id: absolute-urls + min-version: 1.44.0 + severity: WARNING + languages: + - apex + metadata: + category: best-practice + technology: + - salesforce + message: >- + Using absolute URLs to Salesforce Pages is bug prone. Different sandboxes and production + environments will have different instance names (like "na10", "na15" etc.). Code using + absolute URLs will only work when it runs in the corresponding salesforce instances. It + will break as soon as it is deployed in another one. Thus only relative URLs, i.e. without + the domain and subdomain names, should be used when pointing to a salesforce page. + pattern-regex: (http|https)://.*(salesforce|force|visualforce)\.com\.* + paths: + exclude: + - "*Test*" + - "*test*" diff --git a/apex/lang/performance/ncino/operationsInLoops/AvoidNativeDmlInLoops.cls b/apex/lang/performance/ncino/operationsInLoops/AvoidNativeDmlInLoops.cls new file mode 100644 index 0000000000..218d567234 --- /dev/null +++ b/apex/lang/performance/ncino/operationsInLoops/AvoidNativeDmlInLoops.cls @@ -0,0 +1,146 @@ +public class AvoidNativeDmlInLoops { + public void insertInsideLoop() { + for (Integer i = 0; i < 151; i++) { + // ruleid: avoid-native-dml-in-loops + insert account; + // ruleid: avoid-native-dml-in-loops + Database.insert(a); + } + } + + public void insertInsideLoop2() { + for (Account a : accounts) { + // ruleid: avoid-native-dml-in-loops + insert account; + // ruleid: avoid-native-dml-in-loops + Database.insert(a); + } + } + + public void insertInsideLoop3() { + while (someCondition) { + // ruleid: avoid-native-dml-in-loops + insert account; + // ruleid: avoid-native-dml-in-loops + Database.insert(a); + } + } + + public void insertInsideLoop4() { + do { + // ruleid: avoid-native-dml-in-loops + insert account; + // ruleid: avoid-native-dml-in-loops + Database.insert(a); + } while (someCondition); + } + + public void updateInsideLoop() { + for (Integer i = 0; i < 151; i++) { + // ruleid: avoid-native-dml-in-loops + update account; + // ruleid: avoid-native-dml-in-loops + Database.update(a); + } + } + + public void updateInsideLoop2() { + for (Account a : accounts) { + // ruleid: avoid-native-dml-in-loops + update account; + // ruleid: avoid-native-dml-in-loops + Database.update(a); + } + } + + public void updateInsideLoop3() { + while (someCondition) { + // ruleid: avoid-native-dml-in-loops + update account; + // ruleid: avoid-native-dml-in-loops + Database.update(a); + } + } + + public void updateInsideLoop4() { + do { + // ruleid: avoid-native-dml-in-loops + update account; + // ruleid: avoid-native-dml-in-loops + Database.update(a); + } while (someCondition); + } + + public void upsertInsideLoop() { + for (Integer i = 0; i < 151; i++) { + // ruleid: avoid-native-dml-in-loops + upsert account; + // ruleid: avoid-native-dml-in-loops + Database.upsert(a); + } + } + + public void upsertInsideLoop2() { + for (Account a : accounts) { + // ruleid: avoid-native-dml-in-loops + upsert account; + // ruleid: avoid-native-dml-in-loops + Database.upsert(a); + } + } + + public void upsertInsideLoop3() { + while (someCondition) { + // ruleid: avoid-native-dml-in-loops + upsert account; + // ruleid: avoid-native-dml-in-loops + Database.upsert(a); + } + } + + public void upsertInsideLoop4() { + do { + // ruleid: avoid-native-dml-in-loops + upsert account; + // ruleid: avoid-native-dml-in-loops + Database.upsert(a); + } while (someCondition); + } + + public void deleteInsideLoop() { + for (Integer i = 0; i < 151; i++) { + // ruleid: avoid-native-dml-in-loops + delete account; + // ruleid: avoid-native-dml-in-loops + Database.delete(a); + } + } + + public void deleteInsideLoop2() { + for (Account a : accounts) { + // ruleid: avoid-native-dml-in-loops + delete account; + // ruleid: avoid-native-dml-in-loops + Database.delete(a); + } + } + + public void deleteInsideLoop3() { + while (someCondition) { + // ruleid: avoid-native-dml-in-loops + delete account; + // ruleid: avoid-native-dml-in-loops + Database.delete(a); + } + } + + public void deleteInsideLoop4() { + do { + // ruleid: avoid-native-dml-in-loops + delete account; + // ruleid: avoid-native-dml-in-loops + Database.delete(a); + } while (someCondition); + } + +} diff --git a/apex/lang/performance/ncino/operationsInLoops/AvoidNativeDmlInLoops.yaml b/apex/lang/performance/ncino/operationsInLoops/AvoidNativeDmlInLoops.yaml new file mode 100644 index 0000000000..358bcd7cdd --- /dev/null +++ b/apex/lang/performance/ncino/operationsInLoops/AvoidNativeDmlInLoops.yaml @@ -0,0 +1,45 @@ +rules: + - id: avoid-native-dml-in-loops + min-version: 1.44.0 + severity: ERROR + languages: + - generic + metadata: + category: performance + technology: + - salesforce + message: >- + Avoid DML statements inside loops to avoid hitting the DML governor limit. + Instead, try to batch up the data into a list and invoke your DML once on + that list of data outside the loop. + patterns: + - pattern-either: + - pattern-inside: | + for (...) { + ... + } + - pattern-inside: | + while (...) { + ... + } + - pattern-inside: | + do { + ... + } while (...); + - pattern-either: + - pattern: | + insert $DATA; + - pattern: | + update $DATA; + - pattern: | + upsert $DATA; + - pattern: | + delete $DATA; + - pattern: | + Database.insert($DATA); + - pattern: | + Database.update($DATA); + - pattern: | + Database.upsert($DATA); + - pattern: | + Database.delete($DATA); diff --git a/apex/lang/performance/ncino/operationsInLoops/AvoidOperationsWithLimitsInLoops.cls b/apex/lang/performance/ncino/operationsInLoops/AvoidOperationsWithLimitsInLoops.cls new file mode 100644 index 0000000000..fa32ca309a --- /dev/null +++ b/apex/lang/performance/ncino/operationsInLoops/AvoidOperationsWithLimitsInLoops.cls @@ -0,0 +1,121 @@ +public class AvoidOperationsWithLimitsInLoops { + public void messageInsideOfLoop() { + for (Integer i = 0; i < 151; i++) { + Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage(); + // ruleid: avoid-operations-with-limits-in-loops + Messaging.sendEmail(new Messaging.SingleEmailMessage[]{email}); + } + } + + public void messageInsideOfLoop2() { + for (Account a : accounts) { + Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage(); + // ruleid: avoid-operations-with-limits-in-loops + Messaging.sendEmail(new Messaging.SingleEmailMessage[]{email}); + } + } + + public void messageInsideOfLoop3() { + while (someCondition) { + Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage(); + // ruleid: avoid-operations-with-limits-in-loops + Messaging.sendEmail(new Messaging.SingleEmailMessage[]{email}); + } + } + + public void messageInsideOfLoop4() { + do { + Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage(); + // ruleid: avoid-operations-with-limits-in-loops + Messaging.sendEmail(new Messaging.SingleEmailMessage[]{email}); + } while (someCondition); + } + + public void approvalInsideOfLoop() { + for (Integer i = 0; i < 151; i++) { + // ruleid: avoid-operations-with-limits-in-loops + Approval.ProcessSubmitRequest req = new Approval.ProcessSubmitRequest(); + req.setObjectId(acc.Id); + Approval.process(req); + Approval.lock(acc); + Approval.unlock(acc); + } + } + + public void approvalInsideOfLoop2() { + for (Account a : accounts) { + // ruleid: avoid-operations-with-limits-in-loops + Approval.ProcessSubmitRequest req = new Approval.ProcessSubmitRequest(); + req.setObjectId(acc.Id); + Approval.process(req); + Approval.lock(acc); + Approval.unlock(acc); + } + } + + public void approvalInsideOfLoop3() { + while (someCondition) { + // ruleid: avoid-operations-with-limits-in-loops + Approval.ProcessSubmitRequest req = new Approval.ProcessSubmitRequest(); + req.setObjectId(acc.Id); + Approval.process(req); + Approval.lock(acc); + Approval.unlock(acc); + } + } + + public void approvalInsideOfLoop4() { + do { + // ruleid: avoid-operations-with-limits-in-loops + Approval.ProcessSubmitRequest req = new Approval.ProcessSubmitRequest(); + req.setObjectId(acc.Id); + Approval.process(req); + Approval.lock(acc); + Approval.unlock(acc); + } while (someCondition); + } + + public void asyncInsideOfLoop() { + for (Integer i = 0; i < 151; i++) { + // ruleid: avoid-operations-with-limits-in-loops + System.enqueueJob(new MyQueueable()); + // ruleid: avoid-operations-with-limits-in-loops + System.schedule('x', '0 0 0 1 1 ?', new MySchedule()); + // ruleid: avoid-operations-with-limits-in-loops + System.scheduleBatch(new MyBatch(), 'x', 1); + } + } + + public void asyncInsideOfLoop2() { + for (Account a : accounts) { + // ruleid: avoid-operations-with-limits-in-loops + System.enqueueJob(new MyQueueable()); + // ruleid: avoid-operations-with-limits-in-loops + System.schedule('x', '0 0 0 1 1 ?', new MySchedule()); + // ruleid: avoid-operations-with-limits-in-loops + System.scheduleBatch(new MyBatch(), 'x', 1); + } + } + + public void asyncInsideOfLoop3() { + while (someCondition) { + // ruleid: avoid-operations-with-limits-in-loops + System.enqueueJob(new MyQueueable()); + // ruleid: avoid-operations-with-limits-in-loops + System.schedule('x', '0 0 0 1 1 ?', new MySchedule()); + // ruleid: avoid-operations-with-limits-in-loops + System.scheduleBatch(new MyBatch(), 'x', 1); + } + } + + public void asyncInsideOfLoop4() { + do { + // ruleid: avoid-operations-with-limits-in-loops + System.enqueueJob(new MyQueueable()); + // ruleid: avoid-operations-with-limits-in-loops + System.schedule('x', '0 0 0 1 1 ?', new MySchedule()); + // ruleid: avoid-operations-with-limits-in-loops + System.scheduleBatch(new MyBatch(), 'x', 1); + } while (someCondition); + } +} diff --git a/apex/lang/performance/ncino/operationsInLoops/AvoidOperationsWithLimitsInLoops.yaml b/apex/lang/performance/ncino/operationsInLoops/AvoidOperationsWithLimitsInLoops.yaml new file mode 100644 index 0000000000..3325e4b7f6 --- /dev/null +++ b/apex/lang/performance/ncino/operationsInLoops/AvoidOperationsWithLimitsInLoops.yaml @@ -0,0 +1,41 @@ +rules: + - id: avoid-operations-with-limits-in-loops + min-version: 1.44.0 + severity: ERROR + languages: + - generic + metadata: + category: performance + technology: + - salesforce + message: >- + Database class methods, DML operations, SOQL queries, SOSL queries, + Approval class methods, Email sending, async scheduling or queueing + within loops can cause governor limit exceptions. Instead, try to + batch up the data into a list and invoke the operation once on that + list of data outside the loop. + patterns: + - pattern-either: + - pattern-inside: | + for (...) { + ... + } + - pattern-inside: | + while (...) { + ... + } + - pattern-inside: | + do { + ... + } while (...); + - pattern-either: + - pattern: | + Messaging.sendEmail(...); + - pattern: | + Approval.ProcessSubmitRequest $REQUEST = new Approval.ProcessSubmitRequest(); + - pattern: | + System.enqueueJob(...); + - pattern: | + System.schedule(...); + - pattern: | + System.scheduleBatch(...); diff --git a/apex/lang/performance/ncino/operationsInLoops/AvoidSoqlInLoops.cls b/apex/lang/performance/ncino/operationsInLoops/AvoidSoqlInLoops.cls new file mode 100644 index 0000000000..42c06541c6 --- /dev/null +++ b/apex/lang/performance/ncino/operationsInLoops/AvoidSoqlInLoops.cls @@ -0,0 +1,37 @@ +public class AvoidSoqlInLoops { + public void SoqlInsideLoop() { + for (Integer i = 0; i < 151; i++) { + // ruleid: avoid-soql-in-loops + List accounts = [SELECT Id FROM Account]; + // ruleid: avoid-soql-in-loops + Account[] accounts = [SELECT Id FROM Account]; + } + } + + public void SoqlInsideLoop2() { + for (Account a : accounts) { + // ruleid: avoid-soql-in-loops + List accounts = [SELECT Id FROM Account]; + // ruleid: avoid-soql-in-loops + Account[] accounts = [SELECT Id FROM Account]; + } + } + + public void SoqlInsideLoop5() { + while (someCondition) { + // ruleid: avoid-soql-in-loops + List accounts = [SELECT Id FROM Account]; + // ruleid: avoid-soql-in-loops + Account[] accounts = [SELECT Id FROM Account]; + } + } + + public void SoqlInsideLoop7() { + do { + // ruleid: avoid-soql-in-loops + List accounts = [SELECT Id FROM Account]; + // ruleid: avoid-soql-in-loops + Account[] accounts = [SELECT Id FROM Account]; + } while (someCondition); + } +} diff --git a/apex/lang/performance/ncino/operationsInLoops/AvoidSoqlInLoops.yaml b/apex/lang/performance/ncino/operationsInLoops/AvoidSoqlInLoops.yaml new file mode 100644 index 0000000000..e27a7281dd --- /dev/null +++ b/apex/lang/performance/ncino/operationsInLoops/AvoidSoqlInLoops.yaml @@ -0,0 +1,32 @@ +rules: + - id: avoid-soql-in-loops + min-version: 1.44.0 + severity: ERROR + languages: + - generic + metadata: + category: performance + technology: + - salesforce + message: >- + Database class methods, DML operations, SOQL queries, SOSL queries, + Approval class methods, Email sending, async scheduling or queueing + within loops can cause governor limit exceptions. Instead, try to + batch up the data into a list and invoke the operation once on that + list of data outside the loop. + patterns: + - pattern-either: + - pattern-inside: | + for (...) { + ... + } + - pattern-inside: | + while (...) { + ... + } + - pattern-inside: | + do { + ... + } while (...); + - pattern: | + $OBJECTS = [...SELECT...FROM...]; diff --git a/apex/lang/performance/ncino/operationsInLoops/AvoidSoslInLoops.cls b/apex/lang/performance/ncino/operationsInLoops/AvoidSoslInLoops.cls new file mode 100644 index 0000000000..ed28bfca2d --- /dev/null +++ b/apex/lang/performance/ncino/operationsInLoops/AvoidSoslInLoops.cls @@ -0,0 +1,45 @@ +public class AvoidSoslLoops { + public void SoslInsideLoop() { + for (Integer i = 0; i < 151; i++) { + // ruleid: avoid-sosl-in-loops + List objects = (List) Search.query( + 'soslString' + )[0]; + // ruleid: avoid-sosl-in-loops + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } + + public void SoslInsideLoop2() { + for (Account a : accounts) { + // ruleid: avoid-sosl-in-loops + Object[] objects = (Object[]) Search.query( + 'soslString' + )[0]; + // ruleid: avoid-sosl-in-loops + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } + + public void SoslInsideLoop3() { + while (someCondition) { + // ruleid: avoid-sosl-in-loops + List objects = (List) Search.query( + 'soslString' + )[0]; + // ruleid: avoid-sosl-in-loops + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } + + public void SoslInsideLoop4() { + do { + // ruleid: avoid-sosl-in-loops + Object[] objects = (Object[]) Search.query( + 'soslString' + )[0]; + // ruleid: avoid-sosl-in-loops + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } while (someCondition); + } +} diff --git a/apex/lang/performance/ncino/operationsInLoops/AvoidSoslInLoops.yaml b/apex/lang/performance/ncino/operationsInLoops/AvoidSoslInLoops.yaml new file mode 100644 index 0000000000..086f798c17 --- /dev/null +++ b/apex/lang/performance/ncino/operationsInLoops/AvoidSoslInLoops.yaml @@ -0,0 +1,35 @@ +rules: + - id: avoid-sosl-in-loops + min-version: 1.44.0 + severity: ERROR + languages: + - generic + metadata: + category: performance + technology: + - salesforce + message: >- + Database class methods, DML operations, SOQL queries, SOSL queries, + Approval class methods, Email sending, async scheduling or queueing + within loops can cause governor limit exceptions. Instead, try to + batch up the data into a list and invoke the operation once on that + list of data outside the loop. + patterns: + - pattern-either: + - pattern-inside: | + for (...) { + ... + } + - pattern-inside: | + while (...) { + ... + } + - pattern-inside: | + do { + ... + } while (...); + - pattern-either: + - pattern: | + $OBJECTS = ... Search.query(...) + - pattern: | + $OBJECTS = ... [FIND...IN ALL FIELDS RETURNING...] diff --git a/apex/lang/security/ncino/dml/ApexCSRFConstructor.cls b/apex/lang/security/ncino/dml/ApexCSRFConstructor.cls new file mode 100644 index 0000000000..31fb9c981a --- /dev/null +++ b/apex/lang/security/ncino/dml/ApexCSRFConstructor.cls @@ -0,0 +1,55 @@ +/* + * Test Controller with DML in constuctors + */ +public class ApexCSRFConstructor { + + public ApexCSRFConstructor() { + // ruleid: apex-csrf-constructor + insert data; + } + + private ApexCSRFConstructor() { + // ruleid: apex-csrf-constructor + insert data; + } + + public ApexCSRFConstructor() { + // ruleid: apex-csrf-constructor + update data; + } + + private ApexCSRFConstructor() { + // ruleid: apex-csrf-constructor + update data; + } + + public ApexCSRFConstructor() { + // ruleid: apex-csrf-constructor + upsert data; + } + + private ApexCSRFConstructor() { + // ruleid: apex-csrf-constructor + upsert data; + } + + public ApexCSRFConstructor() { + // ruleid: apex-csrf-constructor + delete data; + } + + private ApexCSRFConstructor() { + // ruleid: apex-csrf-constructor + delete data; + } + + public ApexCSRFConstructor() { + // ok: apex-csrf-constructor + notADmlCall(); + } + + private ApexCSRFConstructor() { + // ok: apex-csrf-constructor + notADmlCall(); + } +} diff --git a/apex/lang/security/ncino/dml/ApexCSRFConstructor.yaml b/apex/lang/security/ncino/dml/ApexCSRFConstructor.yaml new file mode 100644 index 0000000000..eb89e2d86e --- /dev/null +++ b/apex/lang/security/ncino/dml/ApexCSRFConstructor.yaml @@ -0,0 +1,51 @@ +rules: + - id: apex-csrf-constructor + min-version: 1.44.0 + severity: ERROR + languages: + - apex + metadata: + cwe: + - 'CWE-352: Cross-Site Request Forgery (CSRF)' + owasp: + - A01:2021 - Broken Access Control + cwe2020-top25': true + cwe2021-top25': true + cwe2022-top25': true + impact: HIGH + likelihood: MEDIUM + confidence: HIGH + category: security + subcategory: + - vuln + technology: + - salesforce + references: + - https://cwe.mitre.org/data/definitions/352.html + message: >- + Having DML operations in Apex class constructor or initializers can + have unexpected side effects: By just accessing a page, the DML statements + would be executed and the database would be modified. Just querying the + database is permitted. + patterns: + - pattern-either: + - pattern-inside: public class $CLASSNAME {...} + - pattern-inside: private class $CLASSNAME {...} + - pattern-inside: public $SOME sharing class $CLASSNAME {...} + - pattern-inside: private $SOME sharing class $CLASSNAME {...} + - pattern-either: + - pattern-inside: public $CLASSNAME() {...} + - pattern-inside: private $CLASSNAME() {...} + - pattern-either: + - pattern: | + insert $DATA; + - pattern: | + update $DATA; + - pattern: | + upsert $DATA; + - pattern: | + delete $DATA; + paths: + exclude: + - "*Test*" + - "*test*" diff --git a/apex/lang/security/ncino/dml/ApexCSRFStaticConstructor.cls b/apex/lang/security/ncino/dml/ApexCSRFStaticConstructor.cls new file mode 100644 index 0000000000..5ca09e5f83 --- /dev/null +++ b/apex/lang/security/ncino/dml/ApexCSRFStaticConstructor.cls @@ -0,0 +1,30 @@ +/* + * Test Controller with DML in constuctors + */ +public class ApexCSRFStaticConstructor { + + static { + // ruleid: apex-csrf-static-constructor + insert data; + } + + static { + // ruleid: apex-csrf-static-constructor + update data; + } + + static { + // ruleid: apex-csrf-static-constructor + upsert data; + } + + static { + // ruleid: apex-csrf-static-constructor + delete data; + } + + static { + // ok: apex-csrf-static-constructor + notADmlCall(); + } +} diff --git a/apex/lang/security/ncino/dml/ApexCSRFStaticConstructor.yaml b/apex/lang/security/ncino/dml/ApexCSRFStaticConstructor.yaml new file mode 100644 index 0000000000..e4706a9434 --- /dev/null +++ b/apex/lang/security/ncino/dml/ApexCSRFStaticConstructor.yaml @@ -0,0 +1,44 @@ +rules: + - id: apex-csrf-static-constructor + min-version: 1.44.0 + severity: ERROR + languages: + - generic + metadata: + cwe: + - 'CWE-352: Cross-Site Request Forgery (CSRF)' + owasp: + - A01:2021 - Broken Access Control + cwe2020-top25': true + cwe2021-top25': true + cwe2022-top25': true + impact: HIGH + likelihood: MEDIUM + confidence: HIGH + category: security + subcategory: + - vuln + technology: + - salesforce + references: + - https://cwe.mitre.org/data/definitions/352.html + message: >- + Having DML operations in Apex class constructor or initializers can + have unexpected side effects: By just accessing a page, the DML statements + would be executed and the database would be modified. Just querying the + database is permitted. + patterns: + - pattern-inside: static {...} + - pattern-either: + - pattern: | + insert $DATA; + - pattern: | + update $DATA; + - pattern: | + upsert $DATA; + - pattern: | + delete $DATA; + paths: + exclude: + - "*Test*" + - "*test*" diff --git a/apex/lang/security/ncino/dml/DmlNativeStatements.cls b/apex/lang/security/ncino/dml/DmlNativeStatements.cls new file mode 100644 index 0000000000..46afacfac1 --- /dev/null +++ b/apex/lang/security/ncino/dml/DmlNativeStatements.cls @@ -0,0 +1,205 @@ +public class DMLNativeStatements { + public void dmlInsert() { + Account act = new Account( + // ok: dml-native-statements + Name = 'insert' + ); + // ruleid: dml-native-statements + insert act; + } + + public void databaseInsert() { + Account act = new Account( + // ok: dml-native-statements + Name = 'insert' + ); + // ruleid: dml-native-statements + Database.insert(act); + } + + public void utilityInsert() { + Account act = new Account( + // ok: dml-native-statements + Name = 'insert' + ); + // ok: dml-native-statements + nFORCE.DMLUtility.insertObj(act); + } + + public void dmlUpsert() { + Account[] acctsList = [ + SELECT + Id, + Name, + BillingCity + FROM + Account + WHERE + BillingCity = 'Bombay' + ]; + + for (Account a : acctsList) { + a.BillingCity = 'Mumbai'; + } + + // ok: dml-native-statements + Account newAcct = new Account(Name = 'upsert', BillingCity = 'San Francisco'); + acctsList.add(newAcct); + + // ruleid: dml-native-statements + upsert acctsList; + } + + public void databaseUpsert() { + Account[] acctsList = [ + SELECT + Id, + Name, + BillingCity + FROM + Account + WHERE + BillingCity = 'Bombay' + ]; + + for (Account a : acctsList) { + a.BillingCity = 'Mumbai'; + } + + // ok: dml-native-statements + Account newAcct = new Account(Name = 'upsert', BillingCity = 'San Francisco'); + acctsList.add(newAcct); + + // ruleid: dml-native-statements + Database.upsert(acctsList); + } + + public void utilityUpsert() { + Account[] acctsList = [ + SELECT + Id, + Name, + BillingCity + FROM + Account + WHERE + BillingCity = 'Bombay' + ]; + + for (Account a : acctsList) { + a.BillingCity = 'Mumbai'; + } + + // ok: dml-native-statements + Account newAcct = new Account(Name = 'upsert', BillingCity = 'San Francisco'); + acctsList.add(newAcct); + + // ok: dml-native-statements + nFORCE.DMLUtility.upsertObjs(acctsList); + } + + public void dmlUpdate() { + Account updtAct = [ + SELECT + BillingCity + FROM + Account + WHERE + // ok: dml-native-statements + Name='update' + LIMIT 1 + ]; + + // ok: dml-native-statements + updtAct.BillingCity = 'update'; + + // ruleid: dml-native-statements + update updtAct; + } + + public void databaseUpdate() { + Account updtAct = [ + SELECT + BillingCity + FROM + Account + WHERE + // ok: dml-native-statements + Name='update' + LIMIT 1 + ]; + + // ok: dml-native-statements + updtAct.BillingCity = 'update'; + + // ruleid: dml-native-statements + Database.update(updtAct); + } + + public void utilityUpdate() { + Account updtAct = [ + SELECT + BillingCity + FROM + Account + WHERE + // ok: dml-native-statements + Name='update' + LIMIT 1 + ]; + + // ok: dml-native-statements + updtAct.BillingCity = 'update'; + + // ok: dml-native-statements + nFORCE.DMLUtility.updateObj(updtAct); + } + + public void dmlDelete() { + Account dltAct = [ + SELECT + Id + FROM + Account + WHERE + // ok: dml-native-statements + Name='delete' + LIMIT 1 + ]; + + // ruleid: dml-native-statements + delete dltAct; + } + + public void databaseDelete() { + Account dltAct = [ + SELECT + Id + FROM + Account + WHERE + // ok: dml-native-statements + Name='delete' + LIMIT 1 + ]; + + // ruleid: dml-native-statements + Database.delete(dltAct); + } + + public void utilityDelete() { + Account dltAct = [ + SELECT + Id + FROM + Account + WHERE + // ok: dml-native-statements + Name='delete' + LIMIT 1 + ]; + + // ok: dml-native-statements + nFORCE.DMLUtility.deleteObj(dltAct); + } +} diff --git a/apex/lang/security/ncino/dml/DmlNativeStatements.yaml b/apex/lang/security/ncino/dml/DmlNativeStatements.yaml new file mode 100644 index 0000000000..3c892d3bd6 --- /dev/null +++ b/apex/lang/security/ncino/dml/DmlNativeStatements.yaml @@ -0,0 +1,35 @@ +rules: + - id: dml-native-statements + min-version: 1.44.0 + severity: WARNING + languages: + - apex + metadata: + cwe: + - 'CWE-863: Incorrect Authorization' + owasp: + - A01:2021 - Broken Access Control + - A04:2021 - Insecure Design + impact: HIGH + likelihood: LOW + confidence: LOW + category: security + subcategory: + - audit + technology: + - salesforce + references: + - https://developer.salesforce.com/docs/atlas.en-us.apexref.meta/apexref/apex_dml_section.htm + - https://cwe.mitre.org/data/definitions/863.html + - https://owasp.org/Top10/A04_2021-Insecure_Design/ + message: >- + Native Salesforce DML operations execute in system context, ignoring the current user's permissions, + field-level security, organization-wide defaults, position in the role hierarchy, and sharing rules. + Be mindful when using native Salesforce DML operations. + patterns: + - pattern-either: + - pattern-regex: '(insert|upsert|update|delete)[\s]' + - pattern-regex: '(insert|upsert|update|delete)[(]' + - pattern-not-regex: '[\/\/].*(insert|upsert|update|delete).*' + - pattern-not-regex: '[\/\/].*(insert|upsert|update|delete)[\n]' + - pattern-not-regex: '.*[=].*(insert|upsert|update|delete).*[,;]' diff --git a/apex/lang/security/ncino/encryption/BadCrypto.cls b/apex/lang/security/ncino/encryption/BadCrypto.cls new file mode 100644 index 0000000000..5c3c94b7ed --- /dev/null +++ b/apex/lang/security/ncino/encryption/BadCrypto.cls @@ -0,0 +1,77 @@ +/* +* Test controller with bad Crypto class usage +*/ +public with sharing class BadCrypto { + + public void badCryptoEncryption() { + // ruleid: bad-crypto + Blob hardCodedIV = Blob.valueOf('Hardcoded IV 123'); + // ruleid: bad-crypto + Blob hardCodedKey = Blob.valueOf('0000000000000000'); + Blob data = Blob.valueOf('Data to be encrypted'); + Blob encrypted = Crypto.encrypt('AES128', hardCodedKey, hardCodedIV, data); + } + + public void badCryptoEncryptionHarcodedIvOnly() { + // ruleid: bad-crypto + Blob hardCodedIV = Blob.valueOf('Hardcoded IV 123'); + // ok: bad-crypto + Blob key = Blob.valueOf(generateEncryptionKey()); + Blob data = Blob.valueOf('Data to be encrypted'); + Blob encrypted = Crypto.encrypt('AES128', key, hardCodedIV, data); + } + + public void badCryptoEncryptionHarcodedKeyOnly() { + // ok: bad-crypto + Blob IV = Blob.valueOf(generateEncryptionIV()); + // ruleid: bad-crypto + Blob hardCodedKey = Blob.valueOf('0000000000000000'); + Blob data = Blob.valueOf('Data to be encrypted'); + Blob encrypted = Crypto.encrypt('AES128', hardCodedKey, IV, data); + } + + public void goodCryptoEncryption() { + // ok: bad-crypto + Blob IV = Blob.valueOf(getRandomValue()); + // ok: bad-crypto + Blob key = Blob.valueOf(getRandomValue()); + Blob data = Blob.valueOf('Data to be encrypted'); + Blob encrypted = Crypto.encrypt('AES128', key, IV, data); + } + + public void badCryptoDecryption() { + Blob encryptedCipherText = Blob.valueOf('Some encrypted cipher text'); + // ruleid: bad-crypto + Blob hardCodedIV = Blob.valueOf('Hardcoded IV 123'); + // ruleid: bad-crypto + Blob hardCodedKey = Blob.valueOf('0000000000000000'); + Blob decryptedCipherText = Crypto.decrypt('AES128', hardCodedKey, hardCodedIV, encryptedCipherText); + } + + public void badCryptoDecryptionHarcodedIvOnly() { + Blob encryptedCipherText = Blob.valueOf('Some encrypted cipher text'); + // ruleid: bad-crypto + Blob hardCodedIV = Blob.valueOf('Hardcoded IV 123'); + // ok: bad-crypto + Blob key = Blob.valueOf(generateEncryptionKey()); + Blob encrypted = Crypto.encrypt('AES128', key, hardCodedIV, data); + } + + public void badCryptoDecryptionHarcodedKeyOnly() { + Blob encryptedCipherText = Blob.valueOf('Some encrypted cipher text'); + // ok: bad-crypto + Blob IV = Blob.valueOf(generateEncryptionIV()); + // ruleid: bad-crypto + Blob hardCodedKey = Blob.valueOf('0000000000000000'); + Blob encrypted = Crypto.encrypt('AES128', hardCodedKey, IV, data); + } + + public void goodCryptoDecryption() { + Blob encryptedCipherText = Blob.valueOf('Some encrypted cipher text'); + // ok: bad-crypto + Blob IV = Blob.valueOf(generateEncryptionIV()); + // ok: bad-crypto + Blob key = Blob.valueOf(getRandomValue()); + Blob encrypted = Crypto.encrypt('AES128', key, IV, data); + } +} diff --git a/apex/lang/security/ncino/encryption/BadCrypto.yaml b/apex/lang/security/ncino/encryption/BadCrypto.yaml new file mode 100644 index 0000000000..38690ec108 --- /dev/null +++ b/apex/lang/security/ncino/encryption/BadCrypto.yaml @@ -0,0 +1,34 @@ +rules: + - id: bad-crypto + min-version: 1.44.0 + severity: ERROR + languages: + - apex + metadata: + cwe: + - 'CWE-321: Use of Hard-coded Cryptographic Key' + owasp: + - A02:2021 - Cryptographic Failures + impact: HIGH + likelihood: LOW + confidence: LOW + category: security + subcategory: + - audit + technology: + - salesforce + references: + - https://cwe.mitre.org/data/definitions/321.html + message: >- + The rule makes sure you are using randomly generated IVs and keys + for Crypto calls. Hard-coding these values greatly compromises the + security of encrypted data. + pattern-either: + - pattern: Blob $IV = Blob.valueOf('$STRING');...Crypto.encrypt($ONE, $TWO, $IV, $FOUR); + - pattern: Blob $IV = Blob.valueOf('$STRING');...Crypto.decrypt($ONE, $TWO, $IV, $FOUR); + - pattern: Blob $KEY = Blob.valueOf('$STRING');...Crypto.encrypt($ONE, $KEY, $THREE, $FOUR); + - pattern: Blob $KEY = Blob.valueOf('$STRING');...Crypto.decrypt($ONE, $KEY, $THREE, $FOUR); + paths: + exclude: + - "*Test*" + - "*test*" diff --git a/apex/lang/security/ncino/endpoints/InsecureHttpRequest.cls b/apex/lang/security/ncino/endpoints/InsecureHttpRequest.cls new file mode 100644 index 0000000000..8d4a241bbc --- /dev/null +++ b/apex/lang/security/ncino/endpoints/InsecureHttpRequest.cls @@ -0,0 +1,54 @@ +/* + * A test controller containing http requests. + * Javadoc insecure endpoint comment test: + * http://www.website.com + * Javadoc secure endpoint comment test: + * https://www.website.com + */ +public with sharing class InsecureHttpRequest { + + // Test comment with insecure endpoint in it + // (comments shouldn't be flagged regardless) + // ok: insecure-http-request + // http://www.website.com + public static void insecureRequestMethod() { + HttpRequest req = new HttpRequest(); + // ruleid: insecure-http-request + req.setEndpoint('http://www.website.com'); + } + + // Test comment with secure endpoint in it + // (comments shouldn't be flagged regardless) + // ok: insecure-http-request + // https://www.website.com + public static void secureRequestMethod() { + HttpRequest req = new HttpRequest(); + // ok: insecure-http-request + req.setEndpoint('https://www.website.com'); + } + + public static void httpRequestUsingLifeCycleCommunicatorConfigObject() { + Communication_Provider__c insecureCommunicator = new Communication_Provider__c( + Name = MY_COMMUNICATOR, + // ruleid: insecure-http-request + Destination__c = 'http://www.website.com', + Billable__c = true, + IsActive__c = true, + Class_Name__c = LifeCycleOAuth2Communicator.class.getName() + ); + + Communication_Provider__c secureCommunicator = new Communication_Provider__c( + Name = MY_COMMUNICATOR, + // ok: insecure-http-request + Destination__c = 'https://www.website.com', + Billable__c = true, + IsActive__c = true, + Class_Name__c = LifeCycleOAuth2Communicator.class.getName() + ); + } + + // ruleid: insecure-http-request + private static final String insecureEndpoint = 'http://www.website.com'; + // ok: insecure-http-request + private static final String secureEndpoint = 'https://www.website.com'; +} diff --git a/apex/lang/security/ncino/endpoints/InsecureHttpRequest.yaml b/apex/lang/security/ncino/endpoints/InsecureHttpRequest.yaml new file mode 100644 index 0000000000..e399c97ecd --- /dev/null +++ b/apex/lang/security/ncino/endpoints/InsecureHttpRequest.yaml @@ -0,0 +1,30 @@ +rules: + - id: insecure-http-request + min-version: 1.44.0 + severity: ERROR + languages: + - apex + metadata: + cwe: + - 'CWE-319: Cleartext Transmission of Sensitive Information' + impact: MEDIUM + likelihood: LOW + confidence: MEDIUM + category: security + subcategory: + - vuln + technology: + - salesforce + references: + - https://cwe.mitre.org/data/definitions/319.html + message: >- + The software transmits sensitive or security-critical data in cleartext in + a communication channel that can be sniffed by unauthorized actors. + patterns: + - pattern-regex: http[:][/][/] + - pattern-not-regex: //.* + - pattern-not-regex: '[*].*' + paths: + exclude: + - "*Test*" + - "*test*" diff --git a/apex/lang/security/ncino/endpoints/NamedCredentialsConstantMatch.cls b/apex/lang/security/ncino/endpoints/NamedCredentialsConstantMatch.cls new file mode 100644 index 0000000000..55c626e13c --- /dev/null +++ b/apex/lang/security/ncino/endpoints/NamedCredentialsConstantMatch.cls @@ -0,0 +1,28 @@ +/* + * Test controller with hard-coded credentials in header + */ +public with sharing class NamedCredentialsConstantMatch { + + public void calloutWithHardcodedCredentials() { + HttpRequest req = new HttpRequest(); + + Blob headerValue = Blob.valueOf('someUsername:somePasswod'); + String authorizationHeader = 'BASIC ' + EncodingUtil.base64Encode(headerValue); + // ruleid: named-credentials-constant-match + req.setHeader(AUTHORIZATION_HEADER, authorizationHeader); + req.setEndpoint('https://www.website.com'); + + HTTPResponse res = new Http().send(req); + } + + public void calloutWithNamedCredential() { + HttpRequest req = new HttpRequest(); + + // ok: named-credentials-constant-match + req.setEndpoint('callout:My_Named_Credential/some_path'); + + HTTPResponse res = new Http().send(req); + } + + public static final String AUTHORIZATION_HEADER = 'Authorization'; +} diff --git a/apex/lang/security/ncino/endpoints/NamedCredentialsConstantMatch.yaml b/apex/lang/security/ncino/endpoints/NamedCredentialsConstantMatch.yaml new file mode 100644 index 0000000000..e3d06ac627 --- /dev/null +++ b/apex/lang/security/ncino/endpoints/NamedCredentialsConstantMatch.yaml @@ -0,0 +1,32 @@ +rules: + - id: named-credentials-constant-match + min-version: 1.44.0 + mode: taint + severity: ERROR + languages: + - apex + metadata: + cwe: + - 'CWE-540: Inclusion of Sensitive Information in Source Code' + impact: HIGH + likelihood: LOW + confidence: HIGH + category: security + subcategory: + - vuln + technology: + - salesforce + references: + - https://cwe.mitre.org/data/definitions/540.html + message: >- + Named Credentials (and callout endpoints) should be used instead of hard-coding credentials. + 1. Hard-coded credentials are hard to maintain when mixed in with application code. + 2. It is particularly hard to update hard-coded credentials when they are used amongst different classes. + 3. Granting a developer access to the codebase means granting knowledge of credentials, and thus keeping a two-level access is not possible. + 4. Using different credentials for different environments is troublesome and error-prone. + pattern-sources: + - pattern: ...String $X = 'Authorization'; + pattern-sinks: + - patterns: + - pattern: req.setHeader($X, ...); + - focus-metavariable: $X diff --git a/apex/lang/security/ncino/endpoints/NamedCredentialsStringMatch.cls b/apex/lang/security/ncino/endpoints/NamedCredentialsStringMatch.cls new file mode 100644 index 0000000000..9f8212effd --- /dev/null +++ b/apex/lang/security/ncino/endpoints/NamedCredentialsStringMatch.cls @@ -0,0 +1,26 @@ +/* + * Test controller with hard-coded credentials in header + */ +public with sharing class NamedCredentialsStringMatch { + + public void calloutWithHardcodedCredentials(String username, String password) { + HttpRequest req = new HttpRequest(); + + Blob headerValue = Blob.valueOf(username + ':' + password); + String authorizationHeader = 'BASIC ' + EncodingUtil.base64Encode(headerValue); + // ruleid: named-credentials-string-match + req.setHeader('Authorization', authorizationHeader); + req.setEndpoint('https://www.website.com'); + + HTTPResponse res = new Http().send(req); + } + + public void calloutWithNamedCredential() { + HttpRequest req = new HttpRequest(); + + // ok: named-credentials-string-match + req.setEndpoint('callout:My_Named_Credential/some_path'); + + HTTPResponse res = new Http().send(req); + } +} diff --git a/apex/lang/security/ncino/endpoints/NamedCredentialsStringMatch.yaml b/apex/lang/security/ncino/endpoints/NamedCredentialsStringMatch.yaml new file mode 100644 index 0000000000..6e8fdd6876 --- /dev/null +++ b/apex/lang/security/ncino/endpoints/NamedCredentialsStringMatch.yaml @@ -0,0 +1,26 @@ +rules: + - id: named-credentials-string-match + min-version: 1.44.0 + severity: ERROR + languages: + - apex + metadata: + cwe: + - 'CWE-540: Inclusion of Sensitive Information in Source Code' + impact: HIGH + likelihood: LOW + confidence: HIGH + category: security + subcategory: + - vuln + technology: + - salesforce + references: + - https://cwe.mitre.org/data/definitions/540.html + message: >- + Named Credentials (and callout endpoints) should be used instead of hard-coding credentials. + 1. Hard-coded credentials are hard to maintain when mixed in with application code. + 2. It is particularly hard to update hard-coded credentials when they are used amongst different classes. + 3. Granting a developer access to the codebase means granting knowledge of credentials, and thus keeping a two-level access is not possible. + 4. Using different credentials for different environments is troublesome and error-prone. + pattern: $REQUEST.setHeader('Authorization', $AUTHSTRING); diff --git a/apex/lang/security/ncino/injection/ApexSOQLInjectionFromUnescapedURLParam.cls b/apex/lang/security/ncino/injection/ApexSOQLInjectionFromUnescapedURLParam.cls new file mode 100644 index 0000000000..88c7aa54a2 --- /dev/null +++ b/apex/lang/security/ncino/injection/ApexSOQLInjectionFromUnescapedURLParam.cls @@ -0,0 +1,39 @@ +/* + * Test controller utilizing unescaped URL params + */ +public class ApexSOQLInjectionFromUnescapedURLParam { + + public void test1() { + String unescapedString = ApexPage.getCurrentPage().getParameters.get('url_param'); + // ruleid: soql-injection-unescaped-url-param + Database.query('SELECT Id FROM Account' + unescapedString); + } + + public void test2() { + String unescapedString = ApexPage.getCurrentPage().getParameters.get('url_param'); + String someQueryClause = someOtherString + unescapedString; + // ruleid: soql-injection-unescaped-url-param + Database.query('SELECT Id FROM Account' + someQueryClause); + } + + public void test3() { + String unescapedString = ApexPage.getCurrentPage().getParameters.get('url_param'); + String escapedString = String.escapeSingleQuotes(unescapedString); + // ok: soql-injection-unescaped-url-param + Database.query('SELECT Id FROM Account' + escapedString); + } + + public void test4() { + String unescapedString = ApexPage.getCurrentPage().getParameters.get('url_param'); + String escapedString = String.escapeSingleQuotes(unescapedString); + String someQueryClause = someOtherString + escapedString; + // ok: soql-injection-unescaped-url-param + Database.query('SELECT Id FROM Account' + someQueryClause); + } + + public void test5() { + String unescapedString = ApexPage.getCurrentPage().getParameters.get('url_param'); + // ok: soql-injection-unescaped-url-param + Database.query('SELECT Id FROM Account' + String.escapeSingleQuotes(unescapedString)); + } +} diff --git a/apex/lang/security/ncino/injection/ApexSOQLInjectionFromUnescapedURLParam.yaml b/apex/lang/security/ncino/injection/ApexSOQLInjectionFromUnescapedURLParam.yaml new file mode 100644 index 0000000000..05f217056f --- /dev/null +++ b/apex/lang/security/ncino/injection/ApexSOQLInjectionFromUnescapedURLParam.yaml @@ -0,0 +1,41 @@ +rules: + - id: soql-injection-unescaped-url-param + min-version: 1.44.0 + mode: taint + severity: ERROR + languages: + - apex + metadata: + category: security + subcategory: + - vuln + technology: + - salesforce + cwe: + - 'CWE-943: Improper Neutralization of Special Elements in Data Query Logic' + owasp: + - A03:2021 - Injection + references: + - https://cwe.mitre.org/data/definitions/943.html + impact: HIGH + likelihood: HIGH + confidence: HIGH + message: >- + If a dynamic query must be used,leverage nFORCE Query Builder. + In other programming languages, the related flaw is known as SQL injection. + Apex doesn't use SQL, but uses its own database query language, SOQL. SOQL is + much simpler and more limited in functionality than SQL. The risks are much + lower for SOQL injection than for SQL injection, but the attacks are nearly + identical to traditional SQL injection. SQL/SOQL injection takes user-supplied + input and uses those values in a dynamic SOQL query. If the input isn't validated, + it can include SOQL commands that effectively modify the SOQL statement and trick + the application into performing unintended commands. + pattern-sources: + - by-side-effect: true + pattern: ApexPage.getCurrentPage().getParameters.get($URLPARAM); + pattern-sanitizers: + - pattern: String.escapeSingleQuotes(...) + pattern-sinks: + - patterns: + - pattern: Database.query($SINK,...); + - focus-metavariable: $SINK diff --git a/apex/lang/security/ncino/injection/ApexSOQLInjectionUnescapedParam.cls b/apex/lang/security/ncino/injection/ApexSOQLInjectionUnescapedParam.cls new file mode 100644 index 0000000000..054bcacdc0 --- /dev/null +++ b/apex/lang/security/ncino/injection/ApexSOQLInjectionUnescapedParam.cls @@ -0,0 +1,63 @@ +public class ApexSOQLInjectionUnescapedParam { + public void test1(String t1) { + // ruleid: soql-injection-unescaped-param + Database.query('SELECT Id FROM Account' + t1); + } + + public void test2(String t1) { + String t2 = t1; + // ruleid: soql-injection-unescaped-param + Database.query('SELECT Id FROM Account' + t2); + } + + public void test3(String t1) { + String escapedStr = String.escapeSingleQuotes(t1); + // ok: soql-injection-unescaped-param + Database.query('SELECT Id FROM Account' + escapedStr); + } + + public void test4(String t1) { + String t2 = t1; + String escapedStr = String.escapeSingleQuotes(t2); + // ok: soql-injection-unescaped-param + Database.query('SELECT Id FROM Account' + escapedStr); + } + + public void test5(Integer t0, String t1, Integer t2) { + // ruleid: soql-injection-unescaped-param + Database.query('SELECT Id FROM Account' + t1); + } + + public void test6(Integer t0, String t1, Integer t2) { + String t2 = t1; + // ruleid: soql-injection-unescaped-param + Database.query('SELECT Id FROM Account' + t2); + } + + public void test7(Integer t0, String t1, Integer t2) { + String escapedStr = String.escapeSingleQuotes(t1); + // ok: soql-injection-unescaped-param + Database.query('SELECT Id FROM Account' + escapedStr); + } + + public void test8(Integer t0, String t1, Integer t2) { + String t2 = t1; + String escapedStr = String.escapeSingleQuotes(t2); + // ok: soql-injection-unescaped-param + Database.query('SELECT Id FROM Account' + escapedStr); + } + + @AuraEnabled(Cacheable=true) + public static List findObjects(String objectName, String searchKey) { + // ok: soql-injection-unescaped-param + List results = Database.query( + 'SELECT Id, Name ' + + 'FROM ' + objectName + ' ' + + 'WHERE Name LIKE \'%' + String.escapeSingleQuotes(searchKey) + '%\' ' + + 'WITH SECURITY_ENFORCED ' + + 'LIMIT 25' + ); + + return results; + } +} diff --git a/apex/lang/security/ncino/injection/ApexSOQLInjectionUnescapedParam.yaml b/apex/lang/security/ncino/injection/ApexSOQLInjectionUnescapedParam.yaml new file mode 100644 index 0000000000..eda51746fa --- /dev/null +++ b/apex/lang/security/ncino/injection/ApexSOQLInjectionUnescapedParam.yaml @@ -0,0 +1,43 @@ +rules: + - id: soql-injection-unescaped-param + min-version: 1.44.0 + mode: taint + severity: ERROR + languages: + - apex + metadata: + category: security + subcategory: + - vuln + technology: + - salesforce + cwe: + - 'CWE-943: Improper Neutralization of Special Elements in Data Query Logic' + owasp: + - A03:2021 - Injection + references: + - https://cwe.mitre.org/data/definitions/943.html + impact: HIGH + likelihood: HIGH + confidence: HIGH + message: >- + If a dynamic query must be used,leverage nFORCE Query Builder. + In other programming languages, the related flaw is known as SQL injection. + Apex doesn't use SQL, but uses its own database query language, SOQL. SOQL is + much simpler and more limited in functionality than SQL. The risks are much + lower for SOQL injection than for SQL injection, but the attacks are nearly + identical to traditional SQL injection. SQL/SOQL injection takes user-supplied + input and uses those values in a dynamic SOQL query. If the input isn't validated, + it can include SOQL commands that effectively modify the SOQL statement and trick + the application into performing unintended commands. + pattern-sources: + - by-side-effect: true + patterns: + - pattern: $M(...,String $P,...) { ... } + - focus-metavariable: $P + pattern-sanitizers: + - pattern-either: + - pattern: String.escapeSingleQuotes($P) + - pattern: Database.query(<... String.escapeSingleQuotes($P) ...>) + pattern-sinks: + - pattern: Database.query(<... $P ...>) diff --git a/apex/lang/security/ncino/sharing/SpecifySharingLevel.cls b/apex/lang/security/ncino/sharing/SpecifySharingLevel.cls new file mode 100644 index 0000000000..02938fc63d --- /dev/null +++ b/apex/lang/security/ncino/sharing/SpecifySharingLevel.cls @@ -0,0 +1,54 @@ +// ruleid: specify-sharing-level +global class SpecifySharingLevel { + // ruleid: specify-sharing-level + public class InnerClass { + public void foo(){ + } + } + + // ruleid: specify-sharing-level + public abstract class InnerClass2 { + public void foo(){ + } + } + + // ruleid: specify-sharing-level + private virtual class InnerClass3 { + public void foo(){ + } + } + + // ok: specify-sharing-level + public inherited sharing class OtherInnerClass { + private void bar(){ + } + } + + // ok: specify-sharing-level + public class SomeExceptionClass extends Exception { + } + + // ok: specify-sharing-level + public with sharing abstract class InnerClass2 { + public void foo(){ + } + } + + // ok: specify-sharing-level + private inherited sharing virtual class InnerClass3 { + public void foo(){ + } + } + + // ok: specify-sharing-level + public inherited sharing virtual class InnerClass3 extends AnotherClass { + public void foo(){ + } + } + + // Observed False Positive + // ok: specify-sharing-level + global virtual override Type forName(String className) { + return Type.forName(className); + } +} diff --git a/apex/lang/security/ncino/sharing/SpecifySharingLevel.yaml b/apex/lang/security/ncino/sharing/SpecifySharingLevel.yaml new file mode 100644 index 0000000000..83cfbcd1ac --- /dev/null +++ b/apex/lang/security/ncino/sharing/SpecifySharingLevel.yaml @@ -0,0 +1,32 @@ +rules: + - id: specify-sharing-level + min-version: 1.44.0 + severity: WARNING + languages: + - apex + metadata: + cwe: + - 'CWE-284: Improper Access Control' + owasp: + - A04:2021 - Insecure Design + references: + - https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_classes_keywords_sharing.htm + - https://cwe.mitre.org/data/definitions/284.html + - https://owasp.org/Top10/A04_2021-Insecure_Design/ + category: security + subcategory: + - vuln + technology: + - salesforce + impact: MEDIUM + likelihood: MEDIUM + confidence: HIGH + message: >- + Every Apex class should have an explicit sharing mode declared. Use the `with sharing` + or `without sharing` keywords on a class to specify whether sharing rules must be enforced. + Use the `inherited sharing` keyword on an Apex class to run the class in the sharing mode + of the class that called it. + patterns: + - pattern-regex: (private|public|global).*\s(class)\s.*[{] + - pattern-not-regex: (private|public|global).*[with|without|inherited]\s[sharing].*\s(class)\s.*[{] + - pattern-not-regex: (private|public|global).*\s(class)\s.*(extends)\s(Exception).*[{] diff --git a/apex/lang/security/ncino/system/SystemDebug.cls b/apex/lang/security/ncino/system/SystemDebug.cls new file mode 100644 index 0000000000..440af2e817 --- /dev/null +++ b/apex/lang/security/ncino/system/SystemDebug.cls @@ -0,0 +1,13 @@ +/* +A test controller containing a debug log. +*/ +public class SystemDebug { + public static void debugMethod() { + // ruleid: system-debug + System.debug('foo'); + } + + public static void noDebugMethod() { + insert new Object(); + } +} diff --git a/apex/lang/security/ncino/system/SystemDebug.yaml b/apex/lang/security/ncino/system/SystemDebug.yaml new file mode 100644 index 0000000000..46978c955b --- /dev/null +++ b/apex/lang/security/ncino/system/SystemDebug.yaml @@ -0,0 +1,30 @@ +rules: + - id: system-debug + min-version: 1.44.0 + severity: WARNING + languages: + - apex + metadata: + cwe: + - 'CWE-489: Active Debug Code' + - 'CWE-779: Logging of Excessive Data' + category: security + subcategory: + - vuln + technology: + - vuln + references: + - https://cwe.mitre.org/data/definitions/489.html + - https://cwe.mitre.org/data/definitions/779.html + impact: MEDIUM + likelihood: LOW + confidence: HIGH + message: >- + In addition to debug statements potentially logging data excessively, debug statements + also contribute to longer transactions and consume Apex CPU time even when debug logs + are not being captured. + pattern: System.debug(...) + paths: + exclude: + - "*Test*" + - "*test*" diff --git a/generic/visualforce/security/ncino/html/UseSRIForCDNs.page b/generic/visualforce/security/ncino/html/UseSRIForCDNs.page new file mode 100644 index 0000000000..2244068677 --- /dev/null +++ b/generic/visualforce/security/ncino/html/UseSRIForCDNs.page @@ -0,0 +1,61 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/generic/visualforce/security/ncino/html/UseSRIForCDNs.yaml b/generic/visualforce/security/ncino/html/UseSRIForCDNs.yaml new file mode 100644 index 0000000000..5f15fbbb48 --- /dev/null +++ b/generic/visualforce/security/ncino/html/UseSRIForCDNs.yaml @@ -0,0 +1,48 @@ +rules: + - id: use-SRI-for-CDNs + languages: + - generic + severity: WARNING + message: >- + Consuming CDNs without including a SubResource Integrity (SRI) can expose your + application and its users to compromised code. SRIs allow you to consume specific + versions of content where if even a single byte is compromised, the resource will + not be loaded. Add an integrity attribute to your + + + diff --git a/generic/visualforce/security/ncino/vf/XSSFromUnescapedURLParam.yaml b/generic/visualforce/security/ncino/vf/XSSFromUnescapedURLParam.yaml new file mode 100644 index 0000000000..529b73921e --- /dev/null +++ b/generic/visualforce/security/ncino/vf/XSSFromUnescapedURLParam.yaml @@ -0,0 +1,47 @@ +rules: + - id: xss-from-unescaped-url-param + languages: + - generic + severity: ERROR + message: >- + To remediate this issue, ensure that all URL parameters are properly + escaped before including them in scripts. Please update your code + to use either the JSENCODE method to escape URL parameters + or the escape="true" attribute on tags. + Passing URL parameters directly into scripts and DOM sinks creates + an opportunity for Cross-Site Scripting attacks. Cross-Site + Scripting (XSS) attacks are a type of injection, in which malicious + scripts are injected into otherwise benign and trusted websites. To + remediate this issue, ensure that all URL parameters are properly + escaped before including them in scripts. + metadata: + cwe: + - "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" + owasp: + - A07:2017 - Cross-Site Scripting (XSS) + - A03:2021 - Injection + references: + - https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/pages_security_tips_xss.htm + category: security + subcategory: + - vuln + technology: + - salesforce + - visualforce + cwe2022-top25: true + cwe2021-top25: true + likelihood: HIGH + impact: MEDIUM + confidence: MEDIUM + patterns: + - pattern-either: + # Cannot use full VF syntax of {!$...} because Semgrep thinks CurrentPage is a metavariable + - pattern: + - pattern: + - pattern: + - pattern-not: + paths: + include: + - "*.component" + - "*.page" + diff --git a/generic/visualforce/security/ncino/xml/CSPHeaderAttribute.page b/generic/visualforce/security/ncino/xml/CSPHeaderAttribute.page new file mode 100644 index 0000000000..fa8a22c7de --- /dev/null +++ b/generic/visualforce/security/ncino/xml/CSPHeaderAttribute.page @@ -0,0 +1,54 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/generic/visualforce/security/ncino/xml/CSPHeaderAttribute.yaml b/generic/visualforce/security/ncino/xml/CSPHeaderAttribute.yaml new file mode 100644 index 0000000000..edcd02bf81 --- /dev/null +++ b/generic/visualforce/security/ncino/xml/CSPHeaderAttribute.yaml @@ -0,0 +1,35 @@ +rules: + - id: csp-header-attribute + languages: + - generic + severity: INFO + message: >- + Visualforce Pages must have the cspHeader attribute set to true. + This attribute is available in API version 55 or higher. + metadata: + cwe: + - "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" + owasp: + - A07:2017 - Cross-Site Scripting (XSS) + - A03:2021 - Injection + references: + - https://help.salesforce.com/s/articleView?id=sf.csp_trusted_sites.htm&type=5 + category: security + subcategory: + - vuln + technology: + - salesforce + - visualforce + cwe2022-top25: true + cwe2021-top25: true + likelihood: HIGH + impact: MEDIUM + confidence: HIGH + patterns: + - pattern: ... + - pattern-not: ... + - pattern-not: ...... + - pattern-not: ...... + paths: + include: + - "*.page" diff --git a/generic/visualforce/security/ncino/xml/VisualForceAPIVersion.page-meta.xml b/generic/visualforce/security/ncino/xml/VisualForceAPIVersion.page-meta.xml new file mode 100644 index 0000000000..872df366ec --- /dev/null +++ b/generic/visualforce/security/ncino/xml/VisualForceAPIVersion.page-meta.xml @@ -0,0 +1,85 @@ + + + + 2.0 + true + false + + + + + 15.0 + true + false + + + + + 20.0 + true + false + + + + + 33.0 + true + false + + + + + 49.0 + true + false + + + + + 50.0 + true + false + + + + + 51.2 + true + false + + + + + 52.0 + true + false + + + + + 53.0 + true + false + + + + + 54.0 + true + false + + + + + 55.0 + true + false + + + + + 62.0 + true + false + + diff --git a/generic/visualforce/security/ncino/xml/VisualForceAPIVersion.yaml b/generic/visualforce/security/ncino/xml/VisualForceAPIVersion.yaml new file mode 100644 index 0000000000..732e730237 --- /dev/null +++ b/generic/visualforce/security/ncino/xml/VisualForceAPIVersion.yaml @@ -0,0 +1,35 @@ +rules: + - id: visualforce-page-api-version + languages: + - generic + severity: WARNING + message: Visualforce Pages must use API version 55 or higher for required use of the cspHeader attribute set to true. + metadata: + cwe: + - "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" + owasp: + - A07:2017 - Cross-Site Scripting (XSS) + - A03:2021 - Injection + references: + - https://developer.salesforce.com/docs/atlas.en-us.api_meta.meta/api_meta/meta_pages.htm + category: security + subcategory: + - vuln + technology: + - salesforce + - visualforce + cwe2022-top25: true + cwe2021-top25: true + likelihood: HIGH + impact: MEDIUM + confidence: HIGH + patterns: + - pattern-inside: + - pattern-either: + - pattern-regex: '[>][0-9].[0-9][<]' + - pattern-regex: '[>][1-4][0-9].[0-9][<]' + - pattern-regex: '[>][5][0-4].[0-9][<]' + paths: + include: + - "*.page-meta.xml" +