Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[False Result] ValidateCrudFlsEmailMessageWhoIdWhatId throws error despite CRUD/FLS checks #1354

Open
thepaulfox opened this issue Feb 9, 2024 · 4 comments
Labels
BUG P3 Rarely Malfunction PMD-AppExchange Issues related to the pmd-appexchange engine rules

Comments

@thepaulfox
Copy link

Salesforce Code Analyzer False Results Template

Description:
We have not found a way through any CRUD or FLS checks to avoid the ValidateCrudFlsEmailMessageWhoIdWhatId error in PMD.

Documentation:

sf scanner run --engine="pmd-appexchange" -t .
Warning: We're continually improving Salesforce Code Analyzer. Tell us what you think! Give feedback at https://research.net/r/SalesforceCA
 Location                                    Description                                        Category                    URL                                                                                                                 
 ─────────────────────────────────────────── ────────────────────────────────────────────────── ─────────────────────────── ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 
 src/classes/ApprovalLwcService.cls:526        Method setTargetObjectId used with whoId/WhatId  AppExchange Security Review https://github.com/forcedotcom/sfdx-scanner/blob/dev/pmd-appexchange/docs/ValidateCrudFlsEmailMessageWhoIdWhatId.md 
                                               with type SingleEmailMessage     

False Results Report:
The rule seems to run on any use of setTargetObjectId on a single email message, regardless of whether we check any access to the field being used. We have tried this with different queries, using stripInaccessible, and other common CRUD/FLS checks.

Steps To Reproduce:
Add the following code to an Apex file:

Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage();
User u = [SELECT Id from User where Id = :UserInfo.getUserId() WITH SECURITY_ENFORCED];
email.setTargetObjectId(u.Id);

Run sf scanner run --engine="pmd-appexchange" -t .

Expected Behavior:
The rule should not fire if the user has read access to the record Id being used. If there is another field related to emails that we should check, the documentation should be updated to include details of what to check.

Screenshots:

Desktop:

  • Operating System. Sonoma 14.1
  • Code Analyzer version. v3.20.0
  • Salesforce CLI version. @salesforce/cli/2.27.6

Additional Context:

Workaround:
Adding //NOPMD on the lines to skip the PMD testing.

Urgency:
Low, we have added this as a false positive for now.

@bobalicious
Copy link

Quick question for clarification - does it highlight the following code in the same way?

Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage();
email.setTargetObjectId(UserInfo.getUserId());

@thepaulfox
Copy link
Author

@bobalicious Yes, it highlights that code the same way.

@rrajaram-salesforce
Copy link
Collaborator

appreciate the feedback @thepaulfox and @bobalicious
Yes, please go ahead with //NOPMD where you believe this is not a vulnerability.

@johnbelosf johnbelosf changed the title ValidateCrudFlsEmailMessageWhoIdWhatId throws error despite CRUD/FLS checks [False Result] ValidateCrudFlsEmailMessageWhoIdWhatId throws error despite CRUD/FLS checks Feb 20, 2024
@johnbelosf johnbelosf added the BUG P3 Rarely Malfunction label Feb 20, 2024
Copy link

git2gus bot commented Feb 20, 2024

This issue has been linked to a new work item: W-15080619

@stephen-carter-at-sf stephen-carter-at-sf added the PMD-AppExchange Issues related to the pmd-appexchange engine rules label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG P3 Rarely Malfunction PMD-AppExchange Issues related to the pmd-appexchange engine rules
Projects
None yet
Development

No branches or pull requests

5 participants