From 05fec8a83768130fd23fc36e297a71dbcfb237c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vili=20Sinerv=C3=A4?= Date: Mon, 25 Nov 2024 16:33:17 +0200 Subject: [PATCH] Add flaw 1: Broken Access Control --- README.md | 14 +++++++++++++- notes/views.py | 8 +++++--- 2 files changed, 18 insertions(+), 4 deletions(-) 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'))