Skip to content

Commit

Permalink
fix unsafe redirect (#308)
Browse files Browse the repository at this point in the history
  • Loading branch information
nitely committed Feb 23, 2022
1 parent 8b48d18 commit 8f32f89
Show file tree
Hide file tree
Showing 15 changed files with 111 additions and 64 deletions.
5 changes: 3 additions & 2 deletions spirit/admin/views.py
@@ -1,12 +1,13 @@
# -*- coding: utf-8 -*-

from django.shortcuts import render, redirect
from django.shortcuts import render
from django.contrib import messages
from django.utils.translation import gettext as _
from django.contrib.auth import get_user_model

import spirit
import django
from spirit.core.utils.http import safe_redirect
from spirit.category.models import Category
from spirit.comment.flag.models import CommentFlag
from spirit.comment.like.models import CommentLike
Expand All @@ -25,7 +26,7 @@ def config_basic(request):
if is_post(request) and form.is_valid():
form.save()
messages.info(request, _("Settings updated!"))
return redirect(request.GET.get("next", request.get_full_path()))
return safe_redirect(request, "next", request.get_full_path())
return render(
request=request,
template_name='spirit/admin/config_basic.html',
Expand Down
7 changes: 4 additions & 3 deletions spirit/comment/flag/views.py
@@ -1,9 +1,10 @@
# -*- coding: utf-8 -*-

from django.contrib.auth.decorators import login_required
from django.shortcuts import render, redirect, get_object_or_404
from django.shortcuts import render, get_object_or_404

from ...core.utils.views import is_post, post_data
from spirit.core.utils.http import safe_redirect
from spirit.core.utils.views import is_post, post_data
from ..models import Comment
from .forms import FlagForm

Expand All @@ -18,7 +19,7 @@ def create(request, comment_id):

if is_post(request) and form.is_valid():
form.save()
return redirect(request.POST.get('next', comment.get_absolute_url()))
return safe_redirect(request, 'next', comment.get_absolute_url(), method='POST')

return render(
request=request,
Expand Down
8 changes: 5 additions & 3 deletions spirit/comment/like/views.py
@@ -1,9 +1,10 @@
# -*- coding: utf-8 -*-

from django.contrib.auth.decorators import login_required
from django.shortcuts import render, redirect, get_object_or_404
from django.shortcuts import render, get_object_or_404
from django.urls import reverse

from spirit.core.utils.http import safe_redirect
from spirit.core.utils.views import is_post, post_data, is_ajax
from spirit.core.utils import json_response
from spirit.comment.models import Comment
Expand All @@ -28,7 +29,7 @@ def create(request, comment_id):
if is_ajax(request):
return json_response({'url_delete': like.get_delete_url()})

return redirect(request.POST.get('next', comment.get_absolute_url()))
return safe_redirect(request, 'next', comment.get_absolute_url(), method='POST')

return render(
request=request,
Expand All @@ -52,7 +53,8 @@ def delete(request, pk):
kwargs={'comment_id': like.comment.pk})
return json_response({'url_create': url, })

return redirect(request.POST.get('next', like.comment.get_absolute_url()))
return safe_redirect(
request, 'next', like.comment.get_absolute_url(), method='POST')

return render(
request=request,
Expand Down
13 changes: 7 additions & 6 deletions spirit/comment/poll/views.py
@@ -1,7 +1,7 @@
# -*- coding: utf-8 -*-

from django.contrib.auth.decorators import login_required
from django.shortcuts import render, redirect, get_object_or_404
from django.shortcuts import render, get_object_or_404
from django.views.decorators.http import require_POST
from django.contrib import messages
from django.contrib.auth.views import redirect_to_login
Expand All @@ -10,8 +10,9 @@

from djconfig import config

from ...core import utils
from ...core.utils.paginator import yt_paginate
from spirit.core.utils.http import safe_redirect
from spirit.core import utils
from spirit.core.utils.paginator import yt_paginate
from .models import CommentPoll, CommentPollChoice, CommentPollVote
from .forms import PollVoteManyForm

Expand All @@ -35,7 +36,7 @@ def close_or_open(request, pk, close=True):
.filter(pk=poll.pk)
.update(close_at=close_at))

return redirect(request.GET.get('next', poll.get_absolute_url()))
return safe_redirect(request, 'next', poll.get_absolute_url())


@require_POST
Expand All @@ -55,10 +56,10 @@ def vote(request, pk):
CommentPollChoice.decrease_vote_count(poll=poll, voter=request.user)
form.save_m2m()
CommentPollChoice.increase_vote_count(poll=poll, voter=request.user)
return redirect(request.POST.get('next', poll.get_absolute_url()))
return safe_redirect(request, 'next', poll.get_absolute_url(), method='POST')

messages.error(request, utils.render_form_errors(form))
return redirect(request.POST.get('next', poll.get_absolute_url()))
return safe_redirect(request, 'next', poll.get_absolute_url(), method='POST')


@login_required
Expand Down
14 changes: 7 additions & 7 deletions spirit/comment/views.py
Expand Up @@ -8,6 +8,7 @@

from djconfig import config

from spirit.core.utils.http import safe_redirect
from spirit.core.utils.views import is_post, post_data, is_ajax
from spirit.core.utils.ratelimit.decorators import ratelimit
from spirit.core.utils.decorators import moderator_required
Expand Down Expand Up @@ -41,15 +42,14 @@ def publish(request, topic_id, pk=None):
if is_post(request) and not request.is_limited() and form.is_valid():
if not user.st.update_post_hash(form.get_comment_hash()):
# Hashed comment may have not been saved yet
return redirect(
request.POST.get('next', None) or
Comment
default_url = lambda: (Comment
.get_last_for_topic(topic_id)
.get_absolute_url())
return safe_redirect(request, 'next', default_url, method='POST')

comment = form.save()
comment_posted(comment=comment, mentions=form.mentions)
return redirect(request.POST.get('next', comment.get_absolute_url()))
return safe_redirect(request, 'next', comment.get_absolute_url(), method='POST')

return render(
request=request,
Expand All @@ -67,7 +67,7 @@ def update(request, pk):
pre_comment_update(comment=Comment.objects.get(pk=comment.pk))
comment = form.save()
post_comment_update(comment=comment)
return redirect(request.POST.get('next', comment.get_absolute_url()))
return safe_redirect(request, 'next', comment.get_absolute_url(), method='POST')
return render(
request=request,
template_name='spirit/comment/update.html',
Expand All @@ -81,7 +81,7 @@ def delete(request, pk, remove=True):
(Comment.objects
.filter(pk=pk)
.update(is_removed=remove))
return redirect(request.GET.get('next', comment.get_absolute_url()))
return safe_redirect(request, 'next', comment.get_absolute_url())
return render(
request=request,
template_name='spirit/comment/moderate.html',
Expand All @@ -104,7 +104,7 @@ def move(request, topic_id):
else:
messages.error(request, render_form_errors(form))

return redirect(request.POST.get('next', topic.get_absolute_url()))
return safe_redirect(request, 'next', topic.get_absolute_url(), method='POST')


def find(request, pk):
Expand Down
4 changes: 2 additions & 2 deletions spirit/core/utils/decorators.py
Expand Up @@ -4,9 +4,9 @@

from django.core.exceptions import PermissionDenied
from django.contrib.auth.views import redirect_to_login
from django.shortcuts import redirect

from spirit.core.conf import settings
from spirit.core.utils.http import safe_redirect


def moderator_required(view_func):
Expand Down Expand Up @@ -48,7 +48,7 @@ def guest_only(view_func):
@wraps(view_func)
def wrapper(request, *args, **kwargs):
if request.user.is_authenticated:
return redirect(request.GET.get('next', request.user.st.get_absolute_url()))
return safe_redirect(request, 'next', request.user.st.get_absolute_url())

return view_func(request, *args, **kwargs)

Expand Down
29 changes: 29 additions & 0 deletions spirit/core/utils/http.py
@@ -0,0 +1,29 @@
# -*- coding: utf-8 -*-

from django.shortcuts import redirect
from django.utils.encoding import iri_to_uri

try:
from django.utils.http import url_has_allowed_host_and_scheme
except ImportError:
from django.utils.http import is_safe_url as url_has_allowed_host_and_scheme


def _resolve_lazy_url(url):
if callable(url):
return url()
return url


def safe_redirect(request, key, default_url='', method='GET'):
next = (
getattr(request, method).get(key, None) or
_resolve_lazy_url(default_url)
)
url_is_safe = url_has_allowed_host_and_scheme(
url=next, allowed_hosts=None)
#allowed_hosts=settings.ALLOWED_HOSTS,
#require_https=request.is_secure())
if url_is_safe:
return redirect(iri_to_uri(next))
return redirect('/')
8 changes: 4 additions & 4 deletions spirit/topic/favorite/views.py
Expand Up @@ -2,14 +2,14 @@

from django.contrib.auth.decorators import login_required
from django.shortcuts import get_object_or_404
from django.shortcuts import redirect
from django.views.decorators.http import require_POST
from django.contrib import messages

from .models import TopicFavorite
from .forms import FavoriteForm
from ..models import Topic
from ...core import utils
from spirit.core import utils
from spirit.core.utils.http import safe_redirect


@require_POST
Expand All @@ -23,12 +23,12 @@ def create(request, topic_id):
else:
messages.error(request, utils.render_form_errors(form))

return redirect(request.POST.get('next', topic.get_absolute_url()))
return safe_redirect(request, 'next', topic.get_absolute_url(), method='POST')


@require_POST
@login_required
def delete(request, pk):
favorite = get_object_or_404(TopicFavorite, pk=pk, user=request.user)
favorite.delete()
return redirect(request.POST.get('next', favorite.topic.get_absolute_url()))
return safe_redirect(request, 'next', favorite.topic.get_absolute_url(), method='POST')
6 changes: 3 additions & 3 deletions spirit/topic/moderate/views.py
@@ -1,10 +1,11 @@
# -*- coding: utf-8 -*-

from django.utils import timezone
from django.shortcuts import render, redirect, get_object_or_404
from django.shortcuts import render, get_object_or_404
from django.contrib import messages
from django.utils.translation import gettext as _

from spirit.core.utils.http import safe_redirect
from spirit.core.utils.views import is_post
from spirit.core.utils.decorators import moderator_required
from spirit.comment.models import Comment
Expand Down Expand Up @@ -33,8 +34,7 @@ def _moderate(request, pk, field_name, to_value, action=None, message=None):
if message is not None:
messages.info(request, message)

return redirect(request.POST.get(
'next', topic.get_absolute_url()))
return safe_redirect(request, 'next', topic.get_absolute_url(), method='POST')

return render(
request=request,
Expand Down
13 changes: 7 additions & 6 deletions spirit/topic/notification/views.py
Expand Up @@ -3,7 +3,7 @@
import json

from django.contrib.auth.decorators import login_required
from django.shortcuts import render, redirect, get_object_or_404
from django.shortcuts import render, get_object_or_404
from django.views.decorators.http import require_POST
from django.http import Http404, HttpResponse
from django.contrib import messages
Expand All @@ -18,6 +18,7 @@
from spirit.core.utils.paginator import yt_paginate
from spirit.core.utils.paginator.infinite_paginator import paginate
from spirit.core.utils.views import is_ajax
from spirit.core.utils.http import safe_redirect
from spirit.topic.models import Topic
from .models import TopicNotification
from .forms import NotificationForm, NotificationCreationForm
Expand All @@ -39,7 +40,7 @@ def create(request, topic_id):
else:
messages.error(request, utils.render_form_errors(form))

return redirect(request.POST.get('next', topic.get_absolute_url()))
return safe_redirect(request, 'next', topic.get_absolute_url(), method='POST')


@require_POST
Expand All @@ -53,8 +54,8 @@ def update(request, pk):
else:
messages.error(request, utils.render_form_errors(form))

return redirect(request.POST.get(
'next', notification.topic.get_absolute_url()))
return safe_redirect(
request, 'next', notification.topic.get_absolute_url(), method='POST')


@login_required
Expand Down Expand Up @@ -124,5 +125,5 @@ def mark_all_as_read(request):
.for_access(request.user)
.filter(is_read=False)
.update(is_read=True))
return redirect(request.POST.get(
'next', reverse('spirit:topic:notification:index')))
return safe_redirect(
request, 'next', reverse('spirit:topic:notification:index'), method='POST')
31 changes: 17 additions & 14 deletions spirit/topic/private/views.py
Expand Up @@ -10,14 +10,15 @@

from djconfig import config

from ...core.conf import settings
from ...core import utils
from ...core.utils.views import is_post, post_data
from ...core.utils.paginator import paginate, yt_paginate
from ...core.utils.ratelimit.decorators import ratelimit
from ...comment.forms import CommentForm
from ...comment.utils import comment_posted
from ...comment.models import Comment
from spirit.core.conf import settings
from spirit.core import utils
from spirit.core.utils.http import safe_redirect
from spirit.core.utils.views import is_post, post_data
from spirit.core.utils.paginator import paginate, yt_paginate
from spirit.core.utils.ratelimit.decorators import ratelimit
from spirit.comment.forms import CommentForm
from spirit.comment.utils import comment_posted
from spirit.comment.models import Comment
from ..models import Topic
from ..utils import topic_viewed
from .utils import notify_access
Expand Down Expand Up @@ -50,9 +51,8 @@ def publish(request, user_id=None):
all([tform.is_valid(), cform.is_valid(), tpform.is_valid()]) and
not request.is_limited()):
if not user.st.update_post_hash(tform.get_topic_hash()):
return redirect(
request.POST.get('next', None) or
tform.category.get_absolute_url())
return safe_redirect(
request, 'next', lambda: tform.category.get_absolute_url(), method='POST')

# wrap in transaction.atomic?
topic = tform.save()
Expand Down Expand Up @@ -123,7 +123,8 @@ def create_access(request, topic_id):
else:
messages.error(request, utils.render_form_errors(form))

return redirect(request.POST.get('next', topic_private.get_absolute_url()))
return safe_redirect(
request, 'next', topic_private.get_absolute_url(), method='POST')


@login_required
Expand All @@ -136,7 +137,8 @@ def delete_access(request, pk):
if request.user.pk == topic_private.user_id:
return redirect(reverse("spirit:topic:private:index"))

return redirect(request.POST.get('next', topic_private.get_absolute_url()))
return safe_redirect(
request, 'next', topic_private.get_absolute_url(), method='POST')

return render(
request=request,
Expand All @@ -160,7 +162,8 @@ def join_in(request, topic_id):
if is_post(request) and form.is_valid():
topic_private = form.save()
notify_access(user=form.get_user(), topic_private=topic_private)
return redirect(request.POST.get('next', topic.get_absolute_url()))
return safe_redirect(
request, 'next', topic.get_absolute_url(), method='POST')
return render(
request=request,
template_name='spirit/topic/private/join.html',
Expand Down

0 comments on commit 8f32f89

Please sign in to comment.