Skip to content

Commit 94658af

Browse files
MashyBaskerunnxt30
authored andcommitted
chore: add checker to detect request.POST method after is_valid() check (DeepSourceCorp#143)
* chore: add checker to detect request.POST method after is_valid() check Signed-off-by: Maharshi Basu <basumaharshi10@gmail.com> * chore: refine checker message Signed-off-by: Maharshi Basu <basumaharshi10@gmail.com> * chore: improve checker message, description + add test case Signed-off-by: Maharshi Basu <basumaharshi10@gmail.com> --------- Signed-off-by: Maharshi Basu <basumaharshi10@gmail.com> Signed-off-by: Unnat Sharma <officialunnat30@gmail.com>
1 parent 2280966 commit 94658af

File tree

2 files changed

+98
-0
lines changed

2 files changed

+98
-0
lines changed
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
from django.shortcuts import render, redirect
2+
from .models import *
3+
from .forms import *
4+
5+
6+
# The idea here is to create an model object called a Tournament, with a form for creation called CreateTournamentForm()
7+
#
8+
# Django best practices are to use form.cleaned_data[] after validation to ensure you're accessing well-sanitized data:
9+
# https://docs.djangoproject.com/en/4.2/ref/forms/api/#accessing-clean-data
10+
#
11+
# Some fairly typical django form object creation handlers
12+
# This handler does NOT use request.cleaned_data[], even after form.is_valid() has run
13+
def create_new_tournament_dangerous(request):
14+
if request.method == 'POST':
15+
form = CreateTournamentForm(request.POST)
16+
if form.is_valid():
17+
# <expect-error>
18+
t = Tournament(name=request.POST['name'])
19+
t.save()
20+
return redirect('index')
21+
else:
22+
context = { 'form': CreateTournamentForm()}
23+
return render(request, 'create_tournament.html', context)
24+
25+
def create_new_tournament_dangerous(request):
26+
if request.method == 'POST':
27+
form = CreateTournamentForm(request.POST)
28+
if form.is_valid():
29+
# <expect-error>
30+
t = Tournament(name=request.POST.get('name'))
31+
t.save()
32+
return redirect('index')
33+
else:
34+
context = { 'form': CreateTournamentForm()}
35+
return render(request, 'create_tournament.html', context)
36+
37+
def create_new_tournament_dangerous_post_after_few_lines(request):
38+
if request.method == 'POST':
39+
form = CreateTournamentForm(request.POST)
40+
if form.is_valid():
41+
# placeholder comment
42+
print("Placeholder print statement")
43+
# <expect-error>
44+
t = Tournament(name=request.POST.get('name'))
45+
t.save()
46+
return redirect('index')
47+
else:
48+
context = { 'form': CreateTournamentForm()}
49+
return render(request, 'create_tournament.html', context)
50+
51+
# This handler DOES use request.cleaned_data[], even after form.is_valid() has run
52+
def create_new_tournament_safe(request):
53+
if request.method == 'POST':
54+
form = CreateTournamentForm(request.POST)
55+
if form.is_valid():
56+
# <no-error>
57+
t = Tournament(name=form.cleaned_data['name'])
58+
t.save()
59+
return redirect('index')
60+
else:
61+
context = { 'form': CreateTournamentForm()}
62+
return render(request, 'create_tournament.html', context)
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
language: py
2+
name: post-after-isvalid
3+
message: Use form.cleaned_data[] after validation instead of request.POST[]
4+
category: security
5+
6+
pattern: |
7+
(subscript
8+
value: (attribute
9+
object: (identifier) @request
10+
attribute: (identifier) @post)
11+
subscript: (_)
12+
(#eq? @request "request")
13+
(#eq? @post "POST")) @post-after-isvalid
14+
15+
(call
16+
function: (attribute
17+
object: (attribute
18+
object: (identifier) @request
19+
attribute: (identifier) @post)
20+
attribute: (identifier) @get)
21+
(#eq? @request "request")
22+
(#eq? @post "POST")
23+
(#eq? @get "get")) @post-after-isvalid
24+
25+
filters:
26+
- pattern-inside: |
27+
(if_statement
28+
condition: (call
29+
function: (attribute
30+
object: (identifier)
31+
attribute: (identifier) @isvalid)
32+
arguments: (argument_list))
33+
(#eq? @isvalid "is_valid"))
34+
35+
description: |
36+
Use form.cleaned_data[] instead of request.POST[] or request.POST.get() after form.is_valid() to ensure data is properly sanitized. Directly accessing request.POST[] can expose the application to security risks, including injection attacks.

0 commit comments

Comments
 (0)