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

Security Policy #353

Open
Otto-AA opened this issue Jun 5, 2022 · 15 comments
Open

Security Policy #353

Otto-AA opened this issue Jun 5, 2022 · 15 comments

Comments

@Otto-AA
Copy link

Otto-AA commented Jun 5, 2022

What is the security policy for this repository? I could not find any info in the README and the Security github section.

In particular, where should I report security vulnerabilities?

@timea-solid
Copy link
Member

@Otto-AA sorry for coming back to you so late.
Feel free to report any issues in this ticket.

@Otto-AA
Copy link
Author

Otto-AA commented Sep 1, 2022

I don't remember where exactly it was in the code, if you need it I can try to dig it up.

Essentially, the markdown renderer is vulnerable to XSS. When viewing a markdown file, arbitrary scripts from this file can be executed. As an example, go to https://sheep.solidcommunity.net/public/ and click on the xss.md file, which will call print() as a XSS demonstration.

As a solution, it is likely enough to go to the documentation of the markdown renderer and follow the security best practices there (again, I forgot which one it is, but they must have a section about filtering scripts for XSS).

@timea-solid
Copy link
Member

@Otto-AA thanks! This already pins it down pretty good. :)

@Otto-AA
Copy link
Author

Otto-AA commented Sep 1, 2022

Also as a general note: restrain from using .innerHtml = ... if not necessary. I've seen it in a number of places, where only text is assigned, not HTML (not sure which one is good, but e.g. .innerText or innerContent should work iirc). This makes it less likely to have XSS

@timea-solid
Copy link
Member

@Otto-AA I think it is clear from your findings we do not have much know-how about security concerns in the code 😓 We could really use some help, even with this simple .innerHtml removal tasks. Would you be interested to join us, take a look? We can also organise a security knowledge transfer so we know what to avoid and fix.
The SolidOs team meets every week on Wednesday 6pm CET more details and you can always come say hi at our gitter chat. We'd like to learn more on security topics and to get to know you.

@bourgeoa
Copy link
Contributor

bourgeoa commented Sep 2, 2022

SoliOS is using https://github.com/markedjs/marked. They recommend to sanitize using https://github.com/cure53/DOMPurify

@timbl
Copy link
Contributor

timbl commented Jan 23, 2023

We should split off the script injection problem from this general "What is your Security Policy" issue.

@Otto-AA
Copy link
Author

Otto-AA commented Feb 9, 2023

I've moved the XSS vulnerability to another issue, so this one can focus on the security policy

@chunt007 chunt007 moved this from Todo to Someday in SolidOS team tasks & Git issues Feb 14, 2023
@Otto-AA
Copy link
Author

Otto-AA commented Apr 13, 2023

@timea-solid I've created a document with basic security recommendations here: https://github.com/Otto-AA/solid-security-basics

If you think it is helpful for you all, I could also join the meeting and talk about this (though I don't know if it would be much more than going through the document). I would have to join late though, as I'm usually not available on Wednesdays before 18:00.

@timea-solid
Copy link
Member

timea-solid commented Apr 13, 2023

@Otto-AA Thank you so so so much! I had this on the back of my mind. I recently learned how to do one myself. So getting us started is so much appreciated.
I would enjoy virtually meeting you but there is no need for you to fight with your schedule. But, please, do join some times :)

And wow may I just say you went above and beyond there: https://github.com/Otto-AA/solid-security-basics

@timea-solid
Copy link
Member

timea-solid commented Apr 13, 2023

@Otto-AA @bourgeoa what do you think about this proposal: #376
@Otto-AA I named you as one of our security champions. Please let me know if this is not comfortable for you.

Context: each and every single repo in SolidOS should have such a policy -> if we settle the working on one, we can than copy it in each repo.

@Otto-AA
Copy link
Author

Otto-AA commented Apr 13, 2023

I would enjoy virtually meeting you but there is no need for you to fight with your schedule. But, please, do join some times :)

Yes, for the next weeks/months I'm only available after 18:00. So I could only join late, or maybe in June/July I'll also have time at 17:30. Let's see :)

And wow may I just say you went above and beyond there: https://github.com/Otto-AA/solid-security-basics

Thanks, I hope it's able to give some useful input!

@Otto-AA I named you as one of our security champions. Please let me know if this is not comfortable for you.

Not necessary, but why not 🤷

And thank you for the enthusiasm, always refreshing to see!

@bourgeoa
Copy link
Contributor

@Otto-AA our meetings have been pushed by 1/2 hour.
From now on they begin at 16h00 UTC that is 18h00 Paris time.

@chunt007
Copy link
Contributor

Weren't we able to sanitize the users input? I didn't know if this issue was fixed or not.

@Otto-AA
Copy link
Author

Otto-AA commented Jan 10, 2024

Weren't we able to sanitize the users input? I didn't know if this issue was fixed or not.

The markdown XSS has moved to a separate issue and has been fixed: #369

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Someday
Development

No branches or pull requests

5 participants