Skip to content

Commit 4497e22

Browse files
Add an additional example and additional test cases for authorize attribute cases
1 parent 475fe3a commit 4497e22

File tree

4 files changed

+63
-3
lines changed

4 files changed

+63
-3
lines changed

csharp/ql/src/Security Features/CWE-639/InsecureDirectObjectReference.qhelp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,17 @@ Ensure that the current user is authorized to access the resource of the provide
1818
<p>In the following example, in the case marked BAD, there is no authorization check, so any user is able to edit any comment.
1919
In the case marked GOOD, there is a check that the current usr matches the author of the comment.</p>
2020
<sample src="WebFormsExample.cs" />
21-
<p>The following example shows a similar case for the ASP.NET Core framework.</p>
21+
<p>The following example shows a similar case for the ASP.NET Core framework. In the third case, the `Authorize` attribute is used
22+
to restrict the method to only administrators, which are expected to be able to access arbitrary resources.
23+
</p>
2224
<sample src="MVCExample.cs" />
2325

24-
2526
</example>
2627
<references>
2728

2829
<li>OWASP - <a href="https://wiki.owasp.org/index.php/Top_10_2013-A4-Insecure_Direct_Object_References">Insecure Direct Object Refrences</a>.</li>
2930
<li>OWASP - <a href="https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/05-Authorization_Testing/04-Testing_for_Insecure_Direct_Object_References">Testing for Insecure Direct Object References</a>.</li>
30-
<li>Microsoft Learn = <a href="https://learn.microsoft.com/en-us/aspnet/core/security/authorization/resourcebased?view=aspnetcore-7.0">Resource-based authorization in ASP.NET Core</a>.</li>
31+
<li>Microsoft Learn - <a href="https://learn.microsoft.com/en-us/aspnet/core/security/authorization/resourcebased?view=aspnetcore-7.0">Resource-based authorization in ASP.NET Core</a>.</li>
3132

3233
</references>
3334
</qhelp>

csharp/ql/src/Security Features/CWE-639/MVCExample.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,14 @@ public async Task<IActionResult> Edit2(int commentId, string text) {
3232
return ForbidResult();
3333
}
3434
}
35+
36+
// GOOD: Only users with the `admin` role can access this method.
37+
[Authorize(Roles="admin")]
38+
public async Task<IActionResult> Edit3(int commentId, string text) {
39+
Comment comment = _commentRepository.Find(commentId);
40+
41+
comment.Text = text;
42+
43+
return View();
44+
}
3545
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
| CommentController.cs:6:25:6:29 | Edit1 | This method may be missing authorization checks for which users can access the resource of the provided ID. |
22
| CommentController.cs:29:25:29:29 | Edit4 | This method may be missing authorization checks for which users can access the resource of the provided ID. |
3+
| MiscTestControllers.cs:26:33:26:40 | EditAnon | This method may be missing authorization checks for which users can access the resource of the provided ID. |
4+
| MiscTestControllers.cs:34:34:34:41 | EditAnon | This method may be missing authorization checks for which users can access the resource of the provided ID. |
5+
| MiscTestControllers.cs:45:25:45:29 | Edit4 | This method may be missing authorization checks for which users can access the resource of the provided ID. |
36
| ProfileController.cs:14:25:14:29 | Edit2 | This method may be missing authorization checks for which users can access the resource of the provided ID. |
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
using Microsoft.AspNetCore.Mvc;
2+
using Microsoft.AspNetCore.Authorization;
3+
4+
public class BaseController : Controller {
5+
// GOOD
6+
[Authorize]
7+
public virtual ActionResult Edit1(int id) { return View(); }
8+
}
9+
10+
class MyAuthorizeAttribute : AuthorizeAttribute { }
11+
class MyAllowAnonymousAttribute : AllowAnonymousAttribute { }
12+
13+
public class AController : BaseController {
14+
// GOOD - Authorize is inherited from overridden method
15+
public override ActionResult Edit1(int id) { return View(); }
16+
17+
// GOOD - A subclass of Authorize is used
18+
[MyAuthorize]
19+
public ActionResult Edit2(int id) { return View(); }
20+
}
21+
22+
[Authorize]
23+
public class BaseAuthController : Controller {
24+
// BAD - A subclass of AllowAnonymous is used
25+
[MyAllowAnonymous]
26+
public virtual ActionResult EditAnon(int id) { return View(); }
27+
}
28+
29+
public class BController : BaseAuthController {
30+
// GOOD - Authorize is inherited from parent class
31+
public ActionResult Edit3(int id) { return View(); }
32+
33+
// BAD - MyAllowAnonymous is inherited from overridden method
34+
public override ActionResult EditAnon(int id) { return View(); }
35+
}
36+
37+
[AllowAnonymous]
38+
public class BaseAnonController : Controller {
39+
40+
}
41+
42+
public class CController : BaseAnonController {
43+
// BAD - AllowAnonymous is inherited from base class and overrides Authorize
44+
[Authorize]
45+
public ActionResult Edit4(int id) { return View(); }
46+
}

0 commit comments

Comments
 (0)