diff --git a/README.md b/README.md index 8c6b5c3..81d1da0 100644 --- a/README.md +++ b/README.md @@ -16,21 +16,33 @@ LINK: https://github.com/VSinerva/csb-project-1 I am using the basic Django template, so no instructions are included. FLAW 1: +> ADD EXACT SOURCE LINK -Broken Access Control (Can delete another user's notes) +(Broken Access Control) Right now, the notes are identified and deleted with a simple sequential ID. +This makes it trivial for a logged in user to delete notes from other users. +The malicious user simply needs to edit the client-side URL of their POST request. + +The issue can easily be fixed by adding the commented out ownership check before deleting a note. +The check compares the logged in user to the owner of the note, and only deletes the note if they match. +This should never cause a problem for a normal user, but it makes sure that the note being deleted +belongs to the logged in user. FLAW 2: +> ADD EXACT SOURCE LINK Cryptographic Failure (Weak/No password hashing) FLAW 3: +> ADD EXACT SOURCE LINK SQL Injection (Unsanitized SQL query for search) FLAW 4: +> ADD EXACT SOURCE LINK Identification and Authentication Failure (No password strength checks) FLAW 5: +> ADD EXACT SOURCE LINK CSRF (No CSRF token for Delete) diff --git a/notes/views.py b/notes/views.py index 767e2f9..e9098a9 100644 --- a/notes/views.py +++ b/notes/views.py @@ -36,10 +36,12 @@ def add(request): @login_required() def remove(request, note_id): if request.method == 'POST': - user = request.user note = Note.objects.get(pk=note_id) - if user == note.owner: - note.delete() +# FLAW 1: +# Adding an ownership check would fix the problem +# user = request.user +# if user == note.owner: + note.delete() return HttpResponseRedirect(request.META.get('HTTP_REFERER', 'index'))