lqez / django-summernote

Simply integrate Summernote editor with Django project.
MIT License
1.04k stars 227 forks source link

Injection Vulnerability using SummernoteInplaceWidget #415

Open Alejandroid17 opened 4 years ago

Alejandroid17 commented 4 years ago

Hi, I have detected that a user can make an xss attack with the data he enters.

How could I solve it?

Reproduce steps

  1. The user enter: <img src=x onerror=alert('XSS');>
  2. When the page is loaded, we will see the alert popup.

Code

from crispy_forms.helper import FormHelper
from crispy_forms.layout import Submit, Column, Row, Layout
from django.forms import HiddenInput
from django.utils.translation import ugettext as _
from django import forms
from django_summernote.widgets import SummernoteInplaceWidget

from myapp.models import MyModel

class MyForm(forms.ModelForm):

    def __init__(self, *args, **kwargs):
        super(MyForm, self).__init__(*args, **kwargs)
        self.helper = FormHelper()
        self.helper.layout = Layout(
            Row(Column('title', css_class='form-group col-md-6'), css_class='form-row'),
            Row(Column('base_template', css_class='form-group col-md-12'), css_class='form-row'),
            Row(Column('base_css_template', css_class='form-group col-md-6', ), css_class='form-row'),
            'doc',
            Submit('submit', _('Save'))
        )

    class Meta:
        model = MyModel
        fields = '__all__'
        widgets = {
            'base_template': SummernoteInplaceWidget(attrs={'summernote': {'width': '100%', 'height': '600px'}}),
            'document_type': HiddenInput()
        }
        labels = {
            'title': _('Title'),
            'base_template': _('Body'),
            'base_css_template': _('CSS stylesheet (optional)'),
            'doc': _('Doc'),
        }
millerthegorilla commented 3 years ago

There is a note about this in the instructions that says something like use 'summernotetextfield' which uses bleach to sanitize html. However, when I use a summernotetextfield on my model, my summernoteinplacewidget starts requiring an iframe, so I guess it has been turned into a summernotewidget, which I am going to open a separate issue for. from the docs....

Last, please don't forget to use safe templatetag while displaying in templates.

{{ foobar|safe }}

Warning: Please mind, that the widget does not provide any escaping. If you expose the widget to external users without taking care of this, it could potentially lead to an injection vulnerability. Therefore you can use the SummernoteTextFormField or SummernoteTextField, which escape all harmful tags through mozilla's package bleach:

In forms,

from django_summernote.fields import SummernoteTextFormField, SummernoteTextField

class SomeForm(forms.Form):
    foo = SummernoteTextFormField()

And for ModelForm,

class FormForSomeModel(forms.ModelForm):
    foo = SummernoteTextField()

because I'm using mozilla on my development I can't view iframes from localhost, so I can't' know if the implementation of bleach is successful. I have tried using bleach separately but it removes tags and attributes, but doesn't seem to handle onerror successfully, although I haven't spent a lot of time on it yet.

millerthegorilla commented 3 years ago

Also, in the latest django_summernote from pypi, there is no bleach at all. from Fields.py in my site-packages:

from django.db import models
from django.forms import fields
from django_summernote.widgets import SummernoteWidget

# code based on https://github.com/shaunsephton/django-ckeditor

class SummernoteTextFormField(fields.CharField):
    def __init__(self, *args, **kwargs):
        kwargs.update({'widget': SummernoteWidget()})
        super(SummernoteTextFormField, self).__init__(*args, **kwargs)

class SummernoteTextField(models.TextField):
    def formfield(self, **kwargs):
        kwargs.update({'form_class': SummernoteTextFormField})
        return super(SummernoteTextField, self).formfield(**kwargs)

and in a directory listing, there is no settings.py, so no bleach at all.

millerthegorilla commented 3 years ago

if its any consolation the same thing happens with django_tinymce. I am guessing the correct answer is to catch the text in the form_valid function and regex out any annoying stuff. Shame bleach doesn't work, although I have opened an issue at https://github.com/mozilla/bleach/issues/582

millerthegorilla commented 3 years ago

Ok, it seems it was my fault. Bleach does work, but not on escaped html. So, I now run

import html

model = form.save(commit=False)
model.text = bleach.clean(html.unescape(model.text), tags=ALLOWED_TAGS, attributes=ATTRIBUTES, styles=STYLES, strip=True, strip_comments=True)
model.save()

and bleach strips the onerror tag as expected.

millerthegorilla commented 3 years ago

Of course, django_summernote has got bleach baked in, at least it will do, when pypi is updated.... :(

shiyunbo commented 3 years ago

It is annoying that the package on Pypi has not included the bleach feature yet.