search

opentechinstitute / luci-commotion

Commotion configuration pages for the LuCI web interface
GNU General Public License v3.0
11 stars 17 forks source link

Splash: Upload field allows invalid input #420

Closed dismantl closed 10 years ago

dismantl commented 10 years ago

The form field for uploading splash page text allows uploading arbitrary files, including binary files which are then displayed without escaping to the user.

dismantl commented 10 years ago

Need some feedback on this. It may not be as important of an issue as I originally thought. The only danger here of arbitrary file upload is for including some malicious javascript or browser exploit. But really there's no way to scan the uploaded file and tell whether any included javascript or markup is malicious or not. We may want to allow node administrators to be able to include javascript in the welcome page text.

Another thing to consider is that you have to have root permissions to be able to upload to this page anyway. But then, an attacker could also upload to this page if they steal a root user's session cookie, and then uploading malicious markup could then harm every user connecting to the access point (assuming welcome page is turned on).

dismantl commented 10 years ago

So I'm ambivalent. Either we leave this as it is, trusting the safety of LuCI's authentication system, or we impose strict limitations on what kind of markup can be included in the uploaded file (e.g. no <script> tags).

Thoughts?

dismantl commented 10 years ago

After discussion with @jheretic, and based on requests we've gotten to allow custom javascript in welcome page markup, we decided it was not feasible to do input validation in this case.