C#: Add query for missing function level access control

[joefarebrother]

Adds query for missing authorization checks.
Finds WebForms methods (System.Web.UI.Page) and MVC actions whose names indicate that they should have authorization checs (e.g. Delete or Admin), but such checks could not be found; including through code, attributes, or Web.config xml files.

[joefarebrother]

Documentation is still in progress, but the technical work is ready for reveiw.

[michaelnebel]

This already looks really good!
I have added some minor comments/recommendations and suggestions.
There primary concern is probably about the use of .calls*(..).
Have you tried running the query on some large projects?

[michaelnebel]

Could this cause performance issues (the reflexive and transitive closure of this could be huge)?

[joefarebrother]

The DCA run looks ok; besides a couple failures that look unrelated to this query

[michaelnebel]

Alright - that sounds good!

[michaelnebel]

@hvitved: Do you have any concerns about using the reflexive, transitive closure of calls?

[hvitved]

If DCA is happy, then I'm happy :-)

[michaelnebel]

This already looks really good! I have added some minor comments/recommendations and suggestions. There primary concern is probably about the use of .calls*(..). Have you tried running the query on some large projects?

It is probably a good idea to run this query against the DCA nightly suite.

[github-actions]

QHelp previews:

csharp/ql/src/Security Features/CWE-285/MissingAccessControl.qhelp

Missing function level access control

Sensitive actions, such as editing or deleting content, or accessing admin pages, should have authorization checks to ensure that they cannot be used by malicious actors.

Recommendation

Ensure that proper authorization checks are made for sensitive actions. For WebForms applications, the authorization tag in Web.config XML files can be used to implement access control. The System.Web.UI.Page.User property can also be used to verify a user's role. For MVC applications, the Authorize attribute can be used to require authorization on specific action methods.

Example

In the following WebForms example, the case marked BAD has no authorization checks whereas the case marked GOOD uses User.IsInRole to check for the user's role.

class ProfilePage : System.Web.UI.Page {
    // BAD: No authorization is used
    protected void btn1_Edit_Click(object sender, EventArgs e) {
        ...
    }

    // GOOD: `User.IsInRole` checks the current user's role.
    protected void btn2_Delete_Click(object sender, EventArgs e) {
        if (!User.IsInRole("admin")) {
            return;
        }
        ...
    }
} 

The following Web.config file uses the authorization tag to deny access to anonymous users, in a location tag to have that configuration apply to a specific path.

<?xml version="1.0"?>

<configuration xmlns:xdt="http://schemas.microsoft.com/XML-Document-Transform">
  <location path="User/Profile">
    <system.web>
      <authorization>
        <deny users="?" />
      </authorization>
    </system.web>
  </location>
</configuration>

In the following MVC example, the case marked BAD has no authorization checks whereas the case marked GOOD uses the Authorize attribute.

public class ProfileController : Controller {

    // BAD: No authorization is used.
    public ActionResult Edit(int id) {
        ...
    }

    // GOOD: The `Authorize` attribute is used.
    [Authorize]
    public ActionResult Delete(int id) {
        ...
    }
}

References

• Page.User Property - Microsoft Learn.
• Control authorization permissions in an ASP.NET application - Microsoft Learn.
• Simple authorization in ASP.NET Core - Microsoft Learn.
• Common Weakness Enumeration: CWE-285.
• Common Weakness Enumeration: CWE-284.
• Common Weakness Enumeration: CWE-862.

[michaelnebel]

Great work Joe!!! :-)
Looks good to me!

[joefarebrother]

Great; just need the docs review then

[tamasvajk]

Looks good, I found two small issues.

[mchammer01]

I'll review this for Docs!

[mchammer01]

@joefarebrother 👋🏻 - I wasn't able to see a preview of the ql file, with the examples in-situ. Have things changed, do you know how to get to the preview?
I reviewed this from an editorial point of view, and left a few comments and suggestions for your consideration. Feel free to ignore the ones you don't agree with 😃
(The punctuation is needed in the list in the References section. )

[joefarebrother]

Thanks @mchammer01 - I've applied those suggestions.
The preview is available in this comment: #13094 (comment)
Possibly it wasn't working previously due to the > typo.

[mchammer01]

@joefarebrother - thanks for pointing me to the preview ✨
I hadn't seen it 🙈
LGTM, just a tiny nit. The // BAD: No authorization is used comment in your first code example is missing a full stop. Feel free to ignore if you think it's OTT 😅

[mchammer01]

LGTM