2

I am baffled by the Salesforce automated security scan. I just ran it and it says:

This query looks for sObject Create operations that are performed without checking for isCreatable(). This may be a false positive if your code accesses only objects whose security is managed by your app and not the admin (for example OAuth states). It may also be a false positive if checks are performed outside of the dataflow (automatically in a visualforce inputfield tag or manually in a constructor), or if this is an enterprise object or other object whose permissions are not set by the admin.

  • Object: profiletoaccount in file: /classes/PrivcoImportController.cls
  • L 60: Account profileAccount = profileToAccount(profile);
  • Object: profileaccount in file: /classes/PrivcoImportController.cls
  • L 66: insert profileAccount;

This is the actual code in my file: 

        if (!profile.containsKey('bad_api_call')) {
            if (!isDuplicateAccount(profile)) {
                Account profileAccount = profileToAccount(profile);                    
                // we need to save this right here, because we need the ID,
                // to link all other items to this account
                try {

                    if (Schema.sObjectType.Account.isCreateable()) {
                        insert profileAccount;
                    } else {
                        SameDayUserMessageModel.addUserMessage('Error! We were not allowed to create the Account Profile. Please talk to the Salesforce admin or manager at your organization, and ask for the appropriate permissions.', 'error');
                    }

So the insert statement is very clearly wrapped by "isCreateable()" and it doesn't seem like this code matches any of the "false positive" conditions that they list. So why would the security scan flag this?

Improve this question
2
  • Can you clarify this bit: it doesn't seem like this code matches any of the "false positive" conditions that they list? Who is "they"? Can you link to the list for context? – Adrian Larson♦ Jul 21 '17 at 20:16
  • "They" is the team at Salesforce who are doing the security review. – LRK9 Jul 24 '17 at 19:44
-3

After a quick review of your code I can tell you that you forget to check isCreateable for fields.

Imagine the situation of a user that can create records for specific object, but he is not allowed to create a record with specific field. You should check isCreateable at both: Object and field level.

isCreateable at DescribeFieldResult: https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_methods_system_fields_describe.htm#isCreateable

Improve this answer
5
  • Obviously I checked isCreatable(). You can plainly see this in the code above. – LRK9 Jul 24 '17 at 19:43
  • As I clearly said, you checked isCreatable at the object level, but not at field level. – D. Cruz Jul 25 '17 at 18:15
  • You are saying I should call isCreatable() on each field? – LRK9 Jul 27 '17 at 19:10
  • No, I'm saying that you have call isCreatable() on each field that will be updated. For example, it doesn't make sense to check isCreatable() on a read only field. I had the same issue, and I pass that validation by doing that. I don't know why people are rating this answer negative if it is a good answer. – D. Cruz Aug 4 '17 at 0:29
  • 1
    People may have taken a negative view of your "answer", because your last comment is a lot more clear than your "answer". – LRK9 Aug 7 '17 at 5:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.