rawpython / remi

Python REMote Interface library. Platform independent. In about 100 Kbytes, perfect for your diet.
Apache License 2.0
3.53k stars 403 forks source link

guideline on safe contents #150

Open bernhardreiter opened 8 years ago

bernhardreiter commented 8 years ago

It seems some contents will be directly put into the HTML, this is a potential security issue if you have any data in your application that comes from somewhere else. For instance, if you are reading a database entry for names.

I've only done a small test using the standard example:

diff --git a/examples/widgets_overview_app.py b/examples/widgets_overview_app.py
index eebe140..3a22779 100644
--- a/examples/widgets_overview_app.py
+++ b/examples/widgets_overview_app.py
@@ -45 +45 @@ class MyApp(App):
-                                   ['104', 'Roberto', 'Robitaille'],
+                                   ['104', 'Roberto <script>alert("hi")</script
@@ -59 +59 @@ class MyApp(App):
-        self.bt = gui.Button('Press me!', width=200, height=30, margin='10px')
+        self.bt = gui.Button('Press <script>alert("button")</script>me!', width
@@ -263 +263 @@ class MyApp(App):
-        self.bt.set_text('Hi!')
+        self.bt.set_text('Hi! <script>alert("Hi!")</script>')

I could "insert" javascript code that got executed, at startup I get the table alert and on the button press the button alert. The set_text somehow did not work for me (I did not inquire further, maybe I've made some mistakes).

The best way to fix this would be to escape contents by default with html.escape(). (And additionally write something about this in the documentation.)

dddomodossola commented 8 years ago

@bernhardreiter the actual structure of the library doesn't allow to escape the contents. This because the App's widgets tree gets entirely represented and sent to the clients. The developer have the freedom to edit the widgets without restrictions, also adding javascript inside. The library itself uses adding javascript inside widgets. In this situation there's no way to protect implictly the user from this kind of vulnerability without affecting the usability and the structure of the library. So we delegate this to the developer that can eventually create an additional layer between the DB and the GUI. I will add a warning inside the readme file in order to inform developers.

bernhardreiter commented 8 years ago

@dddomodossola thanks for the explanation. Adding a warning seems to be a good step then.

Overall I believe in the principle of least surprise, maybe there is a way in the future to see if regular types (like a Python string object) will get rendered with html.escape and there is an additional type like class JavaScriptString(String) that has the full power but need to used explicitly.