[error-report-web] Post/parser: Use bleach to sanitse XSS input

Submitted by Richard Purdie on March 22, 2021, 5:56 p.m. | Patch ID: 179390

Details

Message ID 20210322175626.307889-1-richard.purdie@linuxfoundation.org
State New
Headers show

Commit Message

Richard Purdie March 22, 2021, 5:56 p.m.
Instead of searching for "<", use bleach to sanity input to avoid
any XSS issues.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 Post/parser.py | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

Patch hide | download patch | download mbox

diff --git a/Post/parser.py b/Post/parser.py
index f411e02..536e872 100644
--- a/Post/parser.py
+++ b/Post/parser.py
@@ -9,6 +9,7 @@ 
 # Licensed under the MIT license, see COPYING.MIT for details
 
 import json, re
+import bleach
 from Post.models import Build, BuildFailure, ErrorType
 from django.conf import settings
 from django.utils import timezone
@@ -19,21 +20,6 @@  class Parser:
     def __init__(self, data):
         self.data = data.decode('utf-8')
 
-    # returns true if the values contain '<' char
-    # Ignore the failures field (which is an array anyway)
-    # Ignore any non-str fields too [YOCTO #14208]
-    def contains_tags (self, data):
-        for key,val in data.items():
-            if key == 'failures':
-                continue
-            
-            if not isinstance(val, str):
-                continue
-
-            if '<' in val:
-                return True
-        return False
-
     def parse(self, request):
         build_fails_logged = []
 
@@ -42,8 +28,14 @@  class Parser:
         except:
              return  { 'error' : 'Invalid json' }
 
-        if self.contains_tags(jsondata) == True:
-            return  { 'error' : 'Invalid characters in json' }
+        # Bleach data going directly into the database so that
+        # displaying in any of the graphing doesn't introduce XSS
+        for key,val in jsondata.items():
+            if key == 'failures':
+                continue
+            if not isinstance(val, str):
+                continue
+            jsondata[key] = bleach.clean(val)
 
         b = Build.objects.create()
         try:

Comments

Khem Raj March 23, 2021, 4:25 a.m.
On 3/22/21 10:56 AM, Richard Purdie wrote:
> Instead of searching for "<", use bleach to sanity input to avoid
> any XSS issues.
> 

I will be able to test it. once its installed

> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>   Post/parser.py | 26 +++++++++-----------------
>   1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/Post/parser.py b/Post/parser.py
> index f411e02..536e872 100644
> --- a/Post/parser.py
> +++ b/Post/parser.py
> @@ -9,6 +9,7 @@
>   # Licensed under the MIT license, see COPYING.MIT for details
>   
>   import json, re
> +import bleach
>   from Post.models import Build, BuildFailure, ErrorType
>   from django.conf import settings
>   from django.utils import timezone
> @@ -19,21 +20,6 @@ class Parser:
>       def __init__(self, data):
>           self.data = data.decode('utf-8')
>   
> -    # returns true if the values contain '<' char
> -    # Ignore the failures field (which is an array anyway)
> -    # Ignore any non-str fields too [YOCTO #14208]
> -    def contains_tags (self, data):
> -        for key,val in data.items():
> -            if key == 'failures':
> -                continue
> -
> -            if not isinstance(val, str):
> -                continue
> -
> -            if '<' in val:
> -                return True
> -        return False
> -
>       def parse(self, request):
>           build_fails_logged = []
>   
> @@ -42,8 +28,14 @@ class Parser:
>           except:
>                return  { 'error' : 'Invalid json' }
>   
> -        if self.contains_tags(jsondata) == True:
> -            return  { 'error' : 'Invalid characters in json' }
> +        # Bleach data going directly into the database so that
> +        # displaying in any of the graphing doesn't introduce XSS
> +        for key,val in jsondata.items():
> +            if key == 'failures':
> +                continue
> +            if not isinstance(val, str):
> +                continue
> +            jsondata[key] = bleach.clean(val)
>   
>           b = Build.objects.create()
>           try:
> 
> 
> 
> 
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#52816): https://lists.yoctoproject.org/g/yocto/message/52816
Mute This Topic: https://lists.yoctoproject.org/mt/81531586/3617530
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Yi Fan Yu March 23, 2021, 10:30 p.m.
On 3/22/21 1:56 PM, Richard Purdie wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
> 
> Instead of searching for "<", use bleach to sanity input to avoid
> any XSS issues.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>   Post/parser.py | 26 +++++++++-----------------
>   1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/Post/parser.py b/Post/parser.py
> index f411e02..536e872 100644
> --- a/Post/parser.py
> +++ b/Post/parser.py
> @@ -9,6 +9,7 @@
>   # Licensed under the MIT license, see COPYING.MIT for details
> 
>   import json, re
> +import bleach

would need to add that to `requirements.txt`
i tested it quickly and it gave me bleach not found

>   from Post.models import Build, BuildFailure, ErrorType
>   from django.conf import settings
>   from django.utils import timezone
> @@ -19,21 +20,6 @@ class Parser:
>       def __init__(self, data):
>           self.data = data.decode('utf-8')
> 
> -    # returns true if the values contain '<' char
> -    # Ignore the failures field (which is an array anyway)
> -    # Ignore any non-str fields too [YOCTO #14208]
> -    def contains_tags (self, data):
> -        for key,val in data.items():
> -            if key == 'failures':
> -                continue
> -
> -            if not isinstance(val, str):
> -                continue
> -
> -            if '<' in val:
> -                return True
> -        return False
> -
>       def parse(self, request):
>           build_fails_logged = []
> 
> @@ -42,8 +28,14 @@ class Parser:
>           except:
>                return  { 'error' : 'Invalid json' }
> 
> -        if self.contains_tags(jsondata) == True:
> -            return  { 'error' : 'Invalid characters in json' }
> +        # Bleach data going directly into the database so that
> +        # displaying in any of the graphing doesn't introduce XSS
> +        for key,val in jsondata.items():
> +            if key == 'failures':
> +                continue
> +            if not isinstance(val, str):
> +                continue
> +            jsondata[key] = bleach.clean(val)
> 
would it make more sense to bleach/sanitize the raw data before it got 
parsed into a json object so even 'failures' wouldn't contain any injection

>           b = Build.objects.create()
>           try:
> --
> 2.30.2
> 
> 
> 
> 
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#52828): https://lists.yoctoproject.org/g/yocto/message/52828
Mute This Topic: https://lists.yoctoproject.org/mt/81531586/3617530
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-