Internal
Public Access
Fix HIGH Security Issue: Open Redirect Vulnerability
Fixed open redirect vulnerability in 9 locations throughout tasks/views.py. Changes: - Added url_has_allowed_host_and_scheme import from django.utils.http - Created safe_redirect() helper function to validate redirect URLs - Only allows relative URLs or URLs to the same host - Prevents attackers from redirecting users to phishing/malicious sites Fixed locations: - Line 447: TaskUpdateView - task update redirect - Line 501: task_quick_add - quick add redirect - Line 521: subtask_create - subtask creation redirect - Line 537: task_toggle_status - status toggle redirect - Line 549: task_delete - task deletion redirect - Line 601: web_timer_start - timer start redirect - Line 626: web_timer_stop - timer stop redirect - Line 672: TagUpdateView - tag update redirect - Line 684: tag_delete - tag deletion redirect Security impact: - Prevents phishing attacks via malicious redirect URLs - Blocks cache poisoning attacks - Ensures users stay within the application domain 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.5
parent
0147b8644e
commit
e5b15c7193
+25
-9
@@ -7,6 +7,7 @@ from django.contrib.auth.decorators import login_required
|
||||
from django.db.models import Q
|
||||
from django.shortcuts import render, redirect, get_object_or_404
|
||||
from django.utils import timezone
|
||||
from django.utils.http import url_has_allowed_host_and_scheme
|
||||
from django.views import View
|
||||
from django.views.decorators.http import require_POST
|
||||
from rest_framework import generics, permissions, status, filters
|
||||
@@ -27,6 +28,21 @@ from .permissions import IsOwnerOrReadOnlyIfShared
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def safe_redirect(request, next_url, fallback='dashboard'):
|
||||
"""
|
||||
Safely redirect to next_url after validating it's not an open redirect.
|
||||
|
||||
Only allows relative URLs or URLs to the same host.
|
||||
"""
|
||||
if next_url and url_has_allowed_host_and_scheme(
|
||||
url=next_url,
|
||||
allowed_hosts={request.get_host()},
|
||||
require_https=request.is_secure()
|
||||
):
|
||||
return redirect(next_url)
|
||||
return redirect(fallback)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# API Views
|
||||
# =============================================================================
|
||||
@@ -428,7 +444,7 @@ class TaskDetailView(View):
|
||||
# Redirect to next URL if provided, otherwise back to task detail
|
||||
next_url = request.POST.get('next')
|
||||
if next_url:
|
||||
return redirect(next_url)
|
||||
return safe_redirect(request, next_url, fallback=f'/task/{task.id}/')
|
||||
|
||||
return redirect('task-detail', task_id=task.id)
|
||||
|
||||
@@ -482,7 +498,7 @@ def task_quick_add(request):
|
||||
task.tags.add(tag_id)
|
||||
|
||||
next_url = request.POST.get('next', 'dashboard')
|
||||
return redirect(next_url)
|
||||
return safe_redirect(request, next_url, fallback='dashboard')
|
||||
|
||||
|
||||
@login_required
|
||||
@@ -502,7 +518,7 @@ def subtask_create(request, task_id):
|
||||
subtask.tags.set(parent_task.tags.all())
|
||||
|
||||
next_url = request.POST.get('next', request.META.get('HTTP_REFERER', 'dashboard'))
|
||||
return redirect(next_url)
|
||||
return safe_redirect(request, next_url, fallback='dashboard')
|
||||
|
||||
|
||||
@login_required
|
||||
@@ -518,7 +534,7 @@ def task_toggle_status(request, task_id):
|
||||
task.save()
|
||||
|
||||
next_url = request.POST.get('next', 'dashboard')
|
||||
return redirect(next_url)
|
||||
return safe_redirect(request, next_url, fallback='dashboard')
|
||||
|
||||
|
||||
@login_required
|
||||
@@ -530,7 +546,7 @@ def task_delete(request, task_id):
|
||||
task.save()
|
||||
|
||||
next_url = request.POST.get('next', 'dashboard')
|
||||
return redirect(next_url)
|
||||
return safe_redirect(request, next_url, fallback='dashboard')
|
||||
|
||||
|
||||
@login_required
|
||||
@@ -582,7 +598,7 @@ def web_timer_start(request, task_id):
|
||||
logger.debug(f"Created new timer entry: {entry.id}")
|
||||
|
||||
next_url = request.POST.get('next', 'dashboard')
|
||||
return redirect(next_url)
|
||||
return safe_redirect(request, next_url, fallback='dashboard')
|
||||
|
||||
|
||||
@login_required
|
||||
@@ -607,7 +623,7 @@ def web_timer_stop(request, task_id):
|
||||
logger.debug(f"Stopped {stopped_count} timer entries")
|
||||
|
||||
next_url = request.POST.get('next', 'dashboard')
|
||||
return redirect(next_url)
|
||||
return safe_redirect(request, next_url, fallback='dashboard')
|
||||
|
||||
|
||||
class TagListView(View):
|
||||
@@ -653,7 +669,7 @@ class TagDetailView(View):
|
||||
tag.save()
|
||||
|
||||
next_url = request.POST.get('next', 'tag-list')
|
||||
return redirect(next_url)
|
||||
return safe_redirect(request, next_url, fallback='tag-list')
|
||||
|
||||
|
||||
@login_required
|
||||
@@ -665,4 +681,4 @@ def tag_delete(request, tag_id):
|
||||
tag.save()
|
||||
|
||||
next_url = request.POST.get('next', 'tag-list')
|
||||
return redirect(next_url)
|
||||
return safe_redirect(request, next_url, fallback='tag-list')
|
||||
|
||||
Reference in New Issue
Block a user