2 ms·

Again, whitelist vs blacklist. Blacklist loses. blacklist: [^/?#]+ whitelist: [a-z](?:[0-9a-z]|-(?!(?:$|\.)))(?:\.[a-z](?:[0-9a-z]|-(?!(?:$|\.))))*

by megous 7y ago

Again, whitelist vs blacklist. Blacklist loses. blacklist: [^/?#]+ whitelist: [a-z](?:[0-9a-z]|-(?!(?:$|\.)))*(?:\.[a-z](?:[0-9a-z]|-(?!(?:$|\.)))*)*

robocat 7y ago
Your whitelist loses in this situation? It accepts local domains (e.g. ‘a’) but not fully qualified domain names (e.g. ‘myhost.example.com.’ nor IP addresses. Generally when checking origin, you also wish to check port (e.g. :8000) which the Google code works for; and @username:password may need thought for some mostly obsolete purposes. That said, the whitelist does avoid a bunch of other problems (control chars, Unicode, high bit, etc).
- megous 7y ago
  It was not meant to be complete, just for the domain part. Just like the blacklist regexp was extracted from the domain part too. Local domains are ok.
- robocat 7y ago
  Also, your whitelist is vulnerable to redos DoS attack - according to http://redos-checker.surge.sh/ http://redos-checker.surge.sh/ I’m just trying to point out that securing via Regexps is never simple, even for the very skilled. If someone posts a comment with a Regexp, it is often broken and one can usually find problems with it. Edit: and what are the $ in there for? They look redundant.
  - megous 7y ago
    This is not meant as a complete solution for all the worlds ills, it's just an illustration of the concept and how proper matching would avoid the issue in the OP. Adding \ to the exclusion list just plasters over the pile of garbage that is the original "magic" regexp. $ are there because you can't have '-' at the end of the label. You'd need more chars there if you want to use this as a part of full URL parser. Or just use browser's facilities for parsing URLs. Also noone cares if you can DoS your own browser tab/iframe. You probably can't anyway, because browser has other mechanisms to prevent that. But yes, you'd need to limit this too in real code, to the max length of the label part of the domain name.

Again, whitelist vs blacklist. Blacklist loses. blacklist: [^/?#]+ whitelist: [a-z](?:[0-9a-z]|-(?!(?:$|\.)))*(?:\.[a-z](?:[0-9a-z]|-(?!(?:$|\.)))*)*

by megous 7y ago

Again, whitelist vs blacklist. Blacklist loses. blacklist: [^/?#]+ whitelist: [a-z](?:[0-9a-z]|-(?!(?:$|\.)))*(?:\.[a-z](?:[0-9a-z]|-(?!(?:$|\.)))*)*

robocat 7y ago

Your whitelist loses in this situation? It accepts local domains (e.g. ‘a’) but not fully qualified domain names (e.g. ‘myhost.example.com.’ nor IP addresses. Generally when checking origin, you also wish to check port (e.g. :8000) which the Google code works for; and @username:password may need thought for some mostly obsolete purposes. That said, the whitelist does avoid a bunch of other problems (control chars, Unicode, high bit, etc).

megous 7y ago
It was not meant to be complete, just for the domain part. Just like the blacklist regexp was extracted from the domain part too. Local domains are ok.
robocat 7y ago
Also, your whitelist is vulnerable to redos DoS attack - according to http://redos-checker.surge.sh/ http://redos-checker.surge.sh/ I’m just trying to point out that securing via Regexps is never simple, even for the very skilled. If someone posts a comment with a Regexp, it is often broken and one can usually find problems with it. Edit: and what are the $ in there for? They look redundant.
- megous 7y ago
  This is not meant as a complete solution for all the worlds ills, it's just an illustration of the concept and how proper matching would avoid the issue in the OP. Adding \ to the exclusion list just plasters over the pile of garbage that is the original "magic" regexp. $ are there because you can't have '-' at the end of the label. You'd need more chars there if you want to use this as a part of full URL parser. Or just use browser's facilities for parsing URLs. Also noone cares if you can DoS your own browser tab/iframe. You probably can't anyway, because browser has other mechanisms to prevent that. But yes, you'd need to limit this too in real code, to the max length of the label part of the domain name.