Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions kitsune/community/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from kitsune.search.base import SumoSearchPaginator
from kitsune.search.search import ProfileSearch
from kitsune.sumo.parser import get_object_fallback
from kitsune.sumo.utils import paginate
from kitsune.sumo.utils import paginate, strip_nul_bytes
from kitsune.users.models import ContributionAreas
from kitsune.wiki.models import Document

Expand All @@ -31,7 +31,7 @@ def home(request):

community_news = get_object_fallback(Document, COMMUNITY_NEWS_DOC, request.LANGUAGE_CODE)
locale = _validate_locale(request.GET.get("locale"))
product = request.GET.get("product")
product = strip_nul_bytes(request.GET.get("product"))
if product:
product = get_object_or_404(Product, slug=product)

Expand Down Expand Up @@ -107,7 +107,7 @@ def top_contributors(request, area):
page_size = 100

locale = _validate_locale(request.GET.get("locale"))
product = request.GET.get("product")
product = strip_nul_bytes(request.GET.get("product"))
if product:
product = get_object_or_404(Product, slug=product)

Expand Down
8 changes: 5 additions & 3 deletions kitsune/dashboards/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
)
from kitsune.dashboards.utils import get_locales_by_visit, render_readouts
from kitsune.products.models import Product
from kitsune.sumo.i18n import normalize_language
from kitsune.sumo.urlresolvers import reverse
from kitsune.sumo.utils import smart_int
from kitsune.sumo.utils import smart_int, strip_nul_bytes
from kitsune.wiki.config import CATEGORIES

log = logging.getLogger("k.dashboards")
Expand Down Expand Up @@ -178,11 +179,12 @@ def wiki_rows(request, readout_slug):
"""Return the table contents HTML for the given readout and mode."""
product = _get_product(request)

locale = normalize_language(request.GET.get("locale"))
readout = _kb_readout(
request,
readout_slug,
READOUTS,
locale=request.GET.get("locale"),
locale=locale,
mode=smart_int(request.GET.get("mode"), None),
product=product,
)
Expand Down Expand Up @@ -247,7 +249,7 @@ def aggregated_metrics(request):


def _get_product(request):
product_slug = request.GET.get("product")
product_slug = strip_nul_bytes(request.GET.get("product"))
if product_slug:
return get_object_or_404(Product, slug=product_slug)

Expand Down
1 change: 1 addition & 0 deletions kitsune/flagit/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class FlaggedObject(ModelBase):
(REASON_ABUSE, _lazy("Abusive content")),
(REASON_OTHER, _lazy("Other (please specify)")),
)
VALID_REASONS = frozenset(key for key, _ in REASONS)

FLAG_PENDING = 0
FLAG_ACCEPTED = 1
Expand Down
8 changes: 5 additions & 3 deletions kitsune/flagit/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from kitsune.questions.models import Answer, Question
from kitsune.sumo.templatetags.jinja_helpers import urlparams
from kitsune.sumo.urlresolvers import reverse
from kitsune.sumo.utils import paginate
from kitsune.sumo.utils import paginate, strip_nul_bytes
from kitsune.tags.models import SumoTag


Expand Down Expand Up @@ -132,6 +132,8 @@ def flag(request, content_type=None, model=None, object_id=None, **kwargs):
def flagged_queue(request):
"""Display the flagged queue with optimized queries."""
reason = request.GET.get("reason")
if reason and reason not in FlaggedObject.VALID_REASONS:
reason = None
content_type_id = request.GET.get("content_type")

content_types = None
Expand Down Expand Up @@ -228,8 +230,8 @@ def build_hierarchy(parent_id=None, level=0):
@require_http_methods(["GET", "POST"])
def moderate_content(request):
"""Display flagged content that needs moderation."""
product_slug = request.GET.get("product")
assignee = request.GET.get("assignee")
product_slug = strip_nul_bytes(request.GET.get("product"))
assignee = strip_nul_bytes(request.GET.get("assignee"))

if (
assignee
Expand Down
2 changes: 1 addition & 1 deletion kitsune/gallery/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def get_queryset(self):
# locale may come from the Accept-language header, but it can be
# overridden via the query string.
locale = normalize_language(self.get_locale())
locale = self.request.query_params.get("locale", locale)
locale = normalize_language(self.request.query_params.get("locale")) or locale
if locale is not None:
queryset = queryset.filter(locale=locale)

Expand Down
9 changes: 5 additions & 4 deletions kitsune/gallery/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@
from kitsune.gallery.forms import ImageForm
from kitsune.gallery.models import Image, Video
from kitsune.gallery.utils import check_media_permissions, upload_image
from kitsune.sumo.i18n import normalize_language
from kitsune.sumo.urlresolvers import reverse
from kitsune.sumo.utils import paginate
from kitsune.sumo.utils import paginate, strip_nul_bytes
from kitsune.upload.tasks import compress_image, generate_thumbnail
from kitsune.upload.utils import FileTooLargeError
from kitsune.wiki.tasks import schedule_rebuild_kb
Expand Down Expand Up @@ -114,8 +115,8 @@ def gallery_async(request):
"""
# Maybe refactor this into existing views and check request.is_ajax?
media_type = request.GET.get("type", "image")
term = request.GET.get("q")
media_locale = request.GET.get("locale", settings.WIKI_DEFAULT_LANGUAGE)
term = strip_nul_bytes(request.GET.get("q"))
media_locale = normalize_language(request.GET.get("locale")) or settings.WIKI_DEFAULT_LANGUAGE
if media_type == "image":
media_qs = Image.objects
elif media_type == "video":
Expand All @@ -136,7 +137,7 @@ def gallery_async(request):
def search(request, media_type):
"""Search the media gallery."""

term = request.GET.get("q")
term = strip_nul_bytes(request.GET.get("q", ""))
if not term:
url = reverse("gallery.gallery", args=[media_type])
return HttpResponseRedirect(url)
Expand Down
10 changes: 6 additions & 4 deletions kitsune/kpi/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
)
from kitsune.questions.models import Answer, AnswerVote, Question
from kitsune.sumo.api_utils import OrderingFilter
from kitsune.sumo.i18n import normalize_language
from kitsune.sumo.utils import strip_nul_bytes
from kitsune.wiki.models import HelpfulVote


Expand Down Expand Up @@ -124,8 +126,8 @@ class QuestionsMetricList(CachedAPIView):

def get_objects(self, request):
# Set up the queries for the data we need
locale = request.GET.get("locale")
product = request.GET.get("product")
locale = normalize_language(request.GET.get("locale"))
product = strip_nul_bytes(request.GET.get("product"))

# Set up the query for the data we need.
qs = _daily_qs_for(Question)
Expand Down Expand Up @@ -191,8 +193,8 @@ class KBVoteMetricList(CachedAPIView):

def get_objects(self, request):
# Set up the queries for the data we need
locale = request.GET.get("locale")
product = request.GET.get("product")
locale = normalize_language(request.GET.get("locale"))
product = strip_nul_bytes(request.GET.get("product"))

# Use "__range" to ensure the database index is used in Postgres,
# and only show the helpful votes from the last 365 days.
Expand Down
4 changes: 2 additions & 2 deletions kitsune/messages/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from kitsune.access.decorators import login_required
from kitsune.sumo.decorators import json_view
from kitsune.sumo.utils import webpack_static
from kitsune.sumo.utils import strip_nul_bytes, webpack_static
from kitsune.users.templatetags.jinja_helpers import profile_avatar


Expand All @@ -14,7 +14,7 @@
@json_view
def get_autocomplete_suggestions(request):
"""An API to provide auto-complete data for user names or groups."""
pre = request.GET.get("term", "") or request.GET.get("query", "")
pre = strip_nul_bytes(request.GET.get("term", "") or request.GET.get("query", ""))
if not pre or not request.user.is_authenticated:
return []

Expand Down
3 changes: 2 additions & 1 deletion kitsune/postcrash/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
from django.views.decorators.cache import cache_page

from kitsune.postcrash.models import Signature
from kitsune.sumo.utils import strip_nul_bytes


@cache_page(60 * 60 * 24) # One day.
def api(request):
s = request.GET.get("s", None)
s = strip_nul_bytes(request.GET.get("s", None))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None is not needed in the get() method. It's the default value

if not s:
return HttpResponseBadRequest(content_type="text/plain")

Expand Down
5 changes: 3 additions & 2 deletions kitsune/questions/feeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from kitsune.sumo.feeds import Feed
from kitsune.sumo.templatetags.jinja_helpers import urlparams
from kitsune.sumo.urlresolvers import reverse
from kitsune.sumo.utils import strip_nul_bytes
from kitsune.tags.models import SumoTag


Expand All @@ -27,8 +28,8 @@ class QuestionsFeed(Feed):
def get_object(self, request):
query = {}

product_slug = request.GET.get("product")
topic_slug = request.GET.get("topic")
product_slug = strip_nul_bytes(request.GET.get("product"))
topic_slug = strip_nul_bytes(request.GET.get("topic"))
locale = request.LANGUAGE_CODE

if product_slug and product_slug != "all":
Expand Down
22 changes: 12 additions & 10 deletions kitsune/questions/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
get_mobile_product_from_ua,
)
from kitsune.sumo.decorators import ratelimit
from kitsune.sumo.i18n import normalize_language
from kitsune.sumo.templatetags.jinja_helpers import urlparams
from kitsune.sumo.urlresolvers import reverse
from kitsune.sumo.utils import (
Expand All @@ -69,6 +70,7 @@
paginate,
set_aaq_context,
simple_paginate,
strip_nul_bytes,
)
from kitsune.tags.models import SumoTag
from kitsune.tags.utils import add_existing_tag
Expand Down Expand Up @@ -173,9 +175,9 @@ def question_list(request, product_slug=None, topic_slug=None):
if show not in FILTER_GROUPS:
show = "needs-attention"

tagged = request.GET.get("tagged")
tagged = strip_nul_bytes(request.GET.get("tagged"))
tags = None
topic_slug = request.GET.get("topic", "") or topic_slug
topic_slug = strip_nul_bytes(request.GET.get("topic")) or topic_slug

order = request.GET.get("order", "updated")
if order not in ORDER_BY:
Expand Down Expand Up @@ -587,7 +589,7 @@ def aaq(request, product_slug=None, step=1, is_loginless=False):

# Check if the user is using a mobile device,
# render step 2 if they are
product_slug = product_slug or request.GET.get("product")
product_slug = product_slug or strip_nul_bytes(request.GET.get("product"))
if product_slug is None:
change_product = False
if request.GET.get("q") == "change_product":
Expand Down Expand Up @@ -702,7 +704,7 @@ def aaq(request, product_slug=None, step=1, is_loginless=False):
)

if (
(from_locale := request.GET.get("from_locale"))
(from_locale := normalize_language(request.GET.get("from_locale")))
and from_locale != request.LANGUAGE_CODE
and product.questions_enabled(locale=from_locale)
):
Expand Down Expand Up @@ -1008,7 +1010,7 @@ def solve(request, question_id, answer_id):
"""Accept an answer as the solution to the question."""

if not (
((request.method == "GET") and (watch_secret := request.GET.get("watch", None)))
((request.method == "GET") and (watch_secret := strip_nul_bytes(request.GET.get("watch", None))))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None is not needed in the get() method. It's the default value

or (
(request.method == "POST")
and (request.user.is_authenticated and request.user.is_active)
Expand Down Expand Up @@ -1100,11 +1102,11 @@ def question_vote(request, question_id):
vote.save()

if "referrer" in request.GET:
referrer = request.GET.get("referrer")
referrer = strip_nul_bytes(request.GET.get("referrer"))
vote.add_metadata("referrer", referrer)

if referrer == "search" and "query" in request.GET:
vote.add_metadata("query", request.GET.get("query"))
vote.add_metadata("query", strip_nul_bytes(request.GET.get("query")))

ua = request.META.get("HTTP_USER_AGENT")
if ua:
Expand Down Expand Up @@ -1166,11 +1168,11 @@ def answer_vote(request, question_id, answer_id):
vote.save()

if "referrer" in request.GET:
referrer = request.GET.get("referrer")
referrer = strip_nul_bytes(request.GET.get("referrer"))
vote.add_metadata("referrer", referrer)

if referrer == "search" and "query" in request.GET:
vote.add_metadata("query", request.GET.get("query"))
vote.add_metadata("query", strip_nul_bytes(request.GET.get("query")))

ua = request.META.get("HTTP_USER_AGENT")
if ua:
Expand Down Expand Up @@ -1594,7 +1596,7 @@ def metrics(request, locale_code=None):
"""The Support Forum metrics dashboard."""
template = "questions/metrics.html"

product = request.GET.get("product")
product = strip_nul_bytes(request.GET.get("product"))
if product:
product = get_object_or_404(Product, slug=product)

Expand Down
7 changes: 7 additions & 0 deletions kitsune/sumo/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,13 @@ class TruncationException(Exception):
pass


def strip_nul_bytes(value):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main concern with this PR is that we need to remember to sanitize which leaves a lot of room for errors. For example this PR already missed a call I believe in wiki/views. I suspect this will happen often. We could use a middleware for GET and POST requests only.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@escattone thoughts on this?

"""Strip NUL bytes from a string to prevent database errors."""
if value is None:
return None
return value.replace("\x00", "")


def truncated_json_dumps(obj, max_length, key, ensure_ascii=False):
"""Dump an object to JSON, and ensure the dump is less than ``max_length``.

Expand Down
5 changes: 3 additions & 2 deletions kitsune/users/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from kitsune.questions.utils import num_answers, num_questions, num_solutions
from kitsune.sumo.api_utils import DateTimeUTCField, OrderingFilter, PermissionMod
from kitsune.sumo.decorators import json_view
from kitsune.sumo.utils import strip_nul_bytes
from kitsune.users.models import Profile, Setting
from kitsune.users.templatetags.jinja_helpers import profile_avatar

Expand Down Expand Up @@ -54,8 +55,8 @@ def to_internal_value(self, data):
@json_view
def usernames(request):
"""An API to provide auto-complete data for user names."""
term = request.GET.get("term", "")
query = request.GET.get("query", "")
term = strip_nul_bytes(request.GET.get("term", ""))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some calls like this one, pass an empty string as a default value and others rely on None. We should be consistent

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good way to enforce this is by adding a str -> str contract in the signature of the function

query = strip_nul_bytes(request.GET.get("query", ""))
pre = term or query

if not pre:
Expand Down
11 changes: 6 additions & 5 deletions kitsune/wiki/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from kitsune.products.models import Product, Topic
from kitsune.sumo.api_utils import GenericAPIException, LocaleNegotiationMixin
from kitsune.sumo.i18n import normalize_language
from kitsune.sumo.utils import strip_nul_bytes
from kitsune.wiki.config import REDIRECT_HTML
from kitsune.wiki.models import Document

Expand Down Expand Up @@ -37,26 +38,26 @@ class DocumentList(LocaleNegotiationMixin, generics.ListAPIView):
serializer_class = DocumentShortSerializer

def get_queryset(self):
locales = self.request.query_params.get("locales")
locales = strip_nul_bytes(self.request.query_params.get("locales"))
if locales:
# Multiple locales are separated by commas.
locales = locales.replace(",", " ").split()
elif locale := normalize_language(self.get_locale()):
locales = [locale]

categories = self.request.query_params.get("categories")
categories = strip_nul_bytes(self.request.query_params.get("categories"))
if categories:
# Multiple categories are separated by commas.
categories = categories.replace(",", " ").split()
else:
categories = settings.IA_DEFAULT_CATEGORIES

product = self.request.query_params.get("product")
topic = self.request.query_params.get("topic")
product = strip_nul_bytes(self.request.query_params.get("product"))
topic = strip_nul_bytes(self.request.query_params.get("topic"))
is_template = bool(self.request.query_params.get("is_template", False))
is_archived = bool(self.request.query_params.get("is_archived", False))
is_redirect = bool(self.request.query_params.get("is_redirect", False))
title_query = self.request.query_params.get("q")
title_query = strip_nul_bytes(self.request.query_params.get("q"))

queryset = Document.objects.visible(self.request.user, category__in=categories)

Expand Down
Loading