Issues of the old security audit #20
Description
I noticed there was a security audit and some issues were fixed. I just fixed another simple documentation issue (2.6.).
So @elrido, your last statement about this was:
Most points referring to server side issues have been addressed (that's 2.1, 2.3, 2.7, 2.8). The rest is still open for debate or resolution.
So were any other issues fixed already?
Current state:
- 2.1. Salt and HMAC Key Generated with mt_rand() - fixed
- 2.2. Fixed Server Salt
- 2.3. Traffic Limiter Race Conditions - fixed
- 2.4. VizHash IP Address Online Guessing - fixed
- 2.5. Relies on .htaccess files, which may not be enabled. - Doc addresses this issue
- 2.6. The robots.txt does not work in a subdomain. - fixed
- 2.7 HMAC Not Compared in Constant Time - fixed
- 2.8. Arbitrary File Unlink - fixed
- 2.9. HMAC Uses SHA1 instead of SHA256 - fixed
- 2.10. No Cross-Site Request Forgery Protection - moved to API authentication/CSRF protection #39
- 3.1. Always Assume Malice - partially fixed, ignored, minor issue
- 4.1. Secure Code Delivery - known, unsolveable
- 4.2. urls2links XSS - moved to Remaining theoretical attacks #40
- 4.3. Low Entropy in Browsers Without CSPRNGs - ignored, only affects old clients
- 4.4. Plaintext is Compressed Before Encryption - ignored, not applicable in our case, moved to Make compression optional #38
- 4.5. Is SJCL used correctly? - moved to Remaining theoretical attacks #40
- 4.6. Possible XSS in tpl/page.html - moved to Remaining theoretical attacks #40
I revisited the open points. Here is my opinion on it:
2.2 - Could be solved with a per paste salt.
2.4 - Inherent problem by doing any hashing of the IPs. Add an option to turn vizhash off?
2.9 - Could be changed very easily, but would break all existing delete links. We could make this an option and if enabled also use a per paste salt.
2.10 - We could implement a Cookie-to-Header Token.
3.1 - When I added unit testing I tried to add tests for weird and unexpected strings and subsequently had to improve the input handling. I would like to consider this solved, but there might be something I missed.
4.2 - I haven't found a way to trigger an XSS eighter, but that does not mean that there isn't any. Would consider this to be at least still a risky piece of code.
4.3 - We could add the server based randomness as a fallback. It is mainly an issue with older browsers and on Android
4.4 - If we removed this it would break backwards compatibility, so we would need to make it an option (maybe we should just add a single "support old ZeroBin" option ;-) ).
4.5 - Well... We do use it as per their documentation. Maybe we should ask the SJCL devs if they could have a look and if they had any suggestions for improvements in our usage of it?
4.6 - The JSON and the non encrypted options get generated server side. We should check if one could inject JS that gets executed into a validated encrypted part.
2.4 - Maybe just use another value instead of the IP for the visual hashing.. Let's say, ehm... user agent? I doubt this is better, so... What about:
- just using the entered username? (I'd prefer this)
- using a long-living cookie?
Of course making it optional is a good idea too.
2.9 - Yes, please. When used in HMAC SHA-1 is still secure, but I'd rather prefer SHA-256. MQybe switch this with the switch to a new name?
3.1 - As it also only is a potential attack (as the values are hashed and stuff...) I'd tick it here.
4.2. Use (switch?) to a well-tested RegExp for this?
4.3 - I don't think adding server-randomness is a good idea. And as you said it also only affects older browsers. So I#d tick this as it is obsolete.
4.4 - Also implement when switching to a new name?
4.5 - Yeah, maybe.
4.6. - According to elrido/ZeroBin#84 HTML is correctly encoded there.
Ok, I agree, but I really would like to still offer the possibility for easy upgrade to this fork from the old version. How about making it simple and offer a single option for (less safe) backwards compatibility (disabled by default)? Would also simplify the upgrade wiki page.
Yes, I'm all for options. 😃