-
-
Notifications
You must be signed in to change notification settings - Fork 766
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
Refactor code in current-projects.js to avoid Liquid injection vulnerability #3257
Comments
This comment was marked as resolved.
This comment was marked as resolved.
@tamara-snyder I took a quick look at the issue you wrote since you requested it and it looks like nobody has reviewed this issue yet. First, good job with what you have written so far.
|
Hi @BeckettOBrien, thank you for taking up this issue! Hfla appreciates you :) Do let fellow developers know about your:- You're awesome! P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :) |
Availability for this week: 10 AM to 4 PM weekdays |
@BeckettOBrien When you self-assign an issue (besides your prework checklist), please move it to the "In Progress" column. |
Sorry, done! |
Reopening because the PR to fix this broke the program areas and languages for project cards. I think I know how to fix it so I should be able to have another PR by EOD 7/5/22 (or EOD on whatever day the old PR gets reverted) |
@tamara-snyder I noticed that some of the projects use HTML in their descriptions, which would no longer work after escaping liquid injections. For example, this is what the Undebate project card would look like after this issue is resolved: |
Bonnie asked me to add some details about why this was closed to the issue itself:
List of projects that would be affected: |
Why and where is html usedTo create separate paragraphs
to use a colon (which is a protected character used to define fields)
|
I have detailed where HTML is used in the above referenced files #3257 (comment) Let's talk about other solutions at Sunday's team meeting |
We should try some other solutions or paragraphs, such as solution 1
solution 2
More solutions here: https://stackoverflow.com/questions/3790454/how-do-i-break-a-string-in-yaml-over-multiple-lines |
@BeckettOBrien can you take a look at the solutions provided above and test them out in your local environment to see which might work for us. I believe the issue is we were taking a shortcut, using html in the markdown/yml project files and there are already solutions for this that are suitable for these file types. If we are able to use one of these solutions, we might be able to use the security fix. The one that we can't fix is when the project file uses a colon, and we can replace it with a dash instead. |
I couldn't get any of these solutions to actually produce a line break in the rendered site. I found that the liquid filter |
Putting a link to the decision record for this issue here before its closed: |
Overview
As a developer, I want to keep the sites I work on secure by escaping Liquid injection vulnerabilities so that site remains secure.
Action Items
visible_projects
on line 63.visible_projects
object usingJSON.parse(decodeURIComponent())
and assign it to a variable. You can use lines 62 and 64 (theprojects
object) as a referenceprojects
objectResources/Instructions
Hack for LA Liquid Injection Vulnerabilities Audit
Code file to be edited: assets/js/current-projects.js
Liquid guide
The text was updated successfully, but these errors were encountered: