3 ms·

I don't think it's pedantry. I think it's good to point out that while this change is good, and is in the same realm, if you give the impression that it would p

I don't think it's pedantry. I think it's good to point out that while this change is good, and is in the same realm, if you give the impression that it would prevent an attack like the one this weekend, it's security theater.

pilif 15y ago
It doesn't and I really hoped that I was clear enough in my initial comment. I know that this has nothing to do with the attack on Sunday. But ever since that XSS vulnerability was shown maybe a year ago, I was scared that there could be another one of them as they are very, very easy to miss, especially when you have to rely on filtering instead of escaping. Github has to because you can freely use HTML in Markdown, which is the main markup language on github. So there we are: Relying on filtering out bad stuff from user content, instead of blindly escaping it, at least one XSS vulnerability already happened and the public key interface still allows adding keys without confirming the password. It would really suck to be hit by a XSS attack that silently adds a key to my account, not only giving the attacker the possibility to impersonate me in commits, but, even worse, giving access to my private repositories. Regardless of the fix for the problem on Sunday, I have always hoped Github would add the check for the password. Not to mitigate the mass-assignment problem, but to prevent a possible XSS attack from being used to deal much worse damage. I see this package of changes that we see now happening at Github as a direct result of the Sunday hack, so while we got the mass-assignment fix (of course that had to be fixed), we also finally got the password recheck which we likely would not have gotten without the hack. Hence my "thank you" to @homakov. Not just for uncovering the mass-assignment thing (which is a simple code fix), but also for forcing a change of policy (with negative usability repercussions and thus probably not universally accepted between github internals) elsewhere.

pilif 15y ago

It doesn't and I really hoped that I was clear enough in my initial comment. I know that this has nothing to do with the attack on Sunday. But ever since that XSS vulnerability was shown maybe a year ago, I was scared that there could be another one of them as they are very, very easy to miss, especially when you have to rely on filtering instead of escaping. Github has to because you can freely use HTML in Markdown, which is the main markup language on github. So there we are: Relying on filtering out bad stuff from user content, instead of blindly escaping it, at least one XSS vulnerability already happened and the public key interface still allows adding keys without confirming the password. It would really suck to be hit by a XSS attack that silently adds a key to my account, not only giving the attacker the possibility to impersonate me in commits, but, even worse, giving access to my private repositories. Regardless of the fix for the problem on Sunday, I have always hoped Github would add the check for the password. Not to mitigate the mass-assignment problem, but to prevent a possible XSS attack from being used to deal much worse damage. I see this package of changes that we see now happening at Github as a direct result of the Sunday hack, so while we got the mass-assignment fix (of course that had to be fixed), we also finally got the password recheck which we likely would not have gotten without the hack. Hence my "thank you" to @homakov. Not just for uncovering the mass-assignment thing (which is a simple code fix), but also for forcing a change of policy (with negative usability repercussions and thus probably not universally accepted between github internals) elsewhere.