[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 Chris Steipp cste...@wikimedia.org changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #14 from Chris Steipp cste...@wikimedia.org --- I don't feel like I've been able to give this as thorough a review as I would like (primarily the included libraries), but: * the app is running on a separate server * most parts of the app are only accessible to reasonably trusted users * it's using mostly libraries that are already in use on the cluster * dynamic scanning has an acceptable level of issues reported As long as we get bug 72193 finished soon (which is a bigger problem), then I think we're ok deploying this now. -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #4 from Gerrit Notification Bot gerritad...@wikimedia.org --- Change 165432 had a related patch set uploaded by BryanDavis: Support formatting messages using Parsoid https://gerrit.wikimedia.org/r/165432 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #3 from Gerrit Notification Bot gerritad...@wikimedia.org --- Change 165431 had a related patch set uploaded by BryanDavis: Remove markdown support https://gerrit.wikimedia.org/r/165431 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 Gerrit Notification Bot gerritad...@wikimedia.org changed: What|Removed |Added Status|NEW |PATCH_TO_REVIEW -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #5 from Bryan Davis bda...@wikimedia.org --- (In reply to Chris Steipp from comment #2) Some general comments so far: * The inclusion of script tags in the markdown seems really problematic, and I think needs a better design * It would help if we had a strict content security policy, so that script tags wouldn't be rendered. From an initial look, I think you could get away with just setting Content-Security-Policy: default-src 'self'; img-src 'self' data: and everything would still work. Easy add. I'll post a patch. * You should also set an X-Frame-Options: DENY header Ok. * Cookies should be set with httponly flag. If someone finds an xss, that will keep the session cookie from being (easily) stolen. Another easy one. * You should set the charset to UTF-8, so browsers don't have to guess (and attacks in utf7 won't work). So set a Content-Type:text/html; charset=UTF-8 header. Ok. I am setting meta charset=utf-8/ in my base template, but that isn't seen until the user agent processes the content. * Password.php - don't fall back to the high entropy seed, just throw an exception if the other methods aren't available. It's ok for salts, but for generating random passwords, it's probably bruteforceable. Patch incoming. Due to the number of libraries this pulls in, I have a long way to go in the review, but the markdown is going to take some time. I've posted patches to remove markdown support and instead use Parsoid to provide rich markup support. I will cherry-pick these to the testing server soon. -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #6 from Gerrit Notification Bot gerritad...@wikimedia.org --- Change 165692 had a related patch set uploaded by BryanDavis: Add headers to make attacking the site harder https://gerrit.wikimedia.org/r/165692 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #7 from Gerrit Notification Bot gerritad...@wikimedia.org --- Change 165693 had a related patch set uploaded by BryanDavis: Do not use weak random for password hashing https://gerrit.wikimedia.org/r/165693 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #8 from Chris Steipp cste...@wikimedia.org --- (In reply to Bryan Davis from comment #5) Ok. I am setting meta charset=utf-8/ in my base template, but that isn't seen until the user agent processes the content. I missed that. That should be good enough. -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #11 from Gerrit Notification Bot gerritad...@wikimedia.org --- Change 165692 merged by jenkins-bot: Add headers to make attacking the site harder https://gerrit.wikimedia.org/r/165692 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #10 from Gerrit Notification Bot gerritad...@wikimedia.org --- Change 165432 merged by jenkins-bot: Support formatting messages using Parsoid https://gerrit.wikimedia.org/r/165432 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #9 from Gerrit Notification Bot gerritad...@wikimedia.org --- Change 165431 merged by jenkins-bot: Remove markdown support https://gerrit.wikimedia.org/r/165431 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #12 from Gerrit Notification Bot gerritad...@wikimedia.org --- Change 165693 merged by jenkins-bot: Do not use weak random for password hashing https://gerrit.wikimedia.org/r/165693 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 Bryan Davis bda...@wikimedia.org changed: What|Removed |Added Status|PATCH_TO_REVIEW |NEW --- Comment #13 from Bryan Davis bda...@wikimedia.org --- Patches are all merged and the testing server has been updated. -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 --- Comment #2 from Chris Steipp cste...@wikimedia.org --- Some general comments so far: * The inclusion of script tags in the markdown seems really problematic, and I think needs a better design * It would help if we had a strict content security policy, so that script tags wouldn't be rendered. From an initial look, I think you could get away with just setting Content-Security-Policy: default-src 'self'; img-src 'self' data: and everything would still work. * You should also set an X-Frame-Options: DENY header * Cookies should be set with httponly flag. If someone finds an xss, that will keep the session cookie from being (easily) stolen. * You should set the charset to UTF-8, so browsers don't have to guess (and attacks in utf7 won't work). So set a Content-Type:text/html; charset=UTF-8 header. * Password.php - don't fall back to the high entropy seed, just throw an exception if the other methods aren't available. It's ok for salts, but for generating random passwords, it's probably bruteforceable. Due to the number of libraries this pulls in, I have a long way to go in the review, but the markdown is going to take some time. -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 Bryan Davis bda...@wikimedia.org changed: What|Removed |Added CC||bda...@wikimedia.org Component|General/Unknown |IEG grant review -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 71624] Security review of IEG grant review
https://bugzilla.wikimedia.org/show_bug.cgi?id=71624 Bryan Davis bda...@wikimedia.org changed: What|Removed |Added Priority|Unprioritized |High CC||g...@wikimedia.org, ||sboute...@wikimedia.org Assignee|wikibugs-l@lists.wikimedia. |cste...@wikimedia.org |org | --- Comment #1 from Bryan Davis bda...@wikimedia.org --- The app is based on code base from the Wikimania Scholarships app. Hopefully knowing that will make it easier for Chris to focus his review. -- You are receiving this mail because: You are the assignee for the bug. You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
