Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs could include more advice on securing apps #118

Open
SoniEx2 opened this issue May 5, 2016 · 7 comments
Open

Docs could include more advice on securing apps #118

SoniEx2 opened this issue May 5, 2016 · 7 comments

Comments

@SoniEx2
Copy link

SoniEx2 commented May 5, 2016

This part of the README talks about SQL injection https://github.com/sailorproject/sailor/blob/master/README.md#creating-pages

but I only see XSS.

@Etiene
Copy link
Member

Etiene commented May 5, 2016

Where do you see XSS?

When I talk about SQL injection I mean the methods in the model module that escape the values passed before building the query to pass to the db module. (or not)
(https://github.com/sailorproject/sailor/blob/master/src/sailor/model.lua#L203)
(https://github.com/sailorproject/sailor/blob/master/src/sailor/model.lua#L258)

@SoniEx2
Copy link
Author

SoniEx2 commented May 5, 2016

Ah, I thought you meant user input being used as-is in the resulting HTML...

@felipedaragon
Copy link
Member

felipedaragon commented May 6, 2016

Can this issue be closed? PS: you might want to open another because I heard Robotnik is back in the Emerald Hill :)

@SoniEx2
Copy link
Author

SoniEx2 commented May 6, 2016

@felipedaragon idk if you know this but putting an username in a webpage without escaping HTML elements = XSS, e.g. let's say the username is <script src="some_dangerous_script.js"></script> and you don't escape it, so now your page's HTML looks like Hello <script src="some_dangeours_script.js"></script>, and the browser runs the script.

Nothing in the README seems to indicate page:render() escapes those (and it shouldn't, because being able to dynamically select HTML content like that is useful, instead escaping should be done by the user).

@felipedaragon
Copy link
Member

felipedaragon commented May 6, 2016

@SoniEx2 are you suggesting the creation of a special readme with basic secure coding tips for Sailor users? Some years ago I wrote an article about a dozens of similar scenarios, not just covering XSS but other potential issues: http://seclists.org/fulldisclosure/2014/May/128

@SoniEx2
Copy link
Author

SoniEx2 commented May 6, 2016

I'm suggesting the README should warn about XSS, as it's relevant to the given example.

@felipedaragon
Copy link
Member

@SoniEx2 @Etiene here is an example of how another framework (Django) tackled this in their documentation: https://docs.djangoproject.com/en/1.9/topics/security/

@felipedaragon felipedaragon changed the title README seems to talk about XSS as SQL injection? Docs could include more advice on securing Sailor-powered sites May 7, 2016
@felipedaragon felipedaragon changed the title Docs could include more advice on securing Sailor-powered sites Docs could include more advice on securing apps May 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants