koajs / csrf

CSRF tokens for koa
MIT License
264 stars 32 forks source link

Using of semicolon in csrf token? #4

Closed remoe closed 10 years ago

remoe commented 10 years ago

Why is a semicolon used for the csrf token here:

https://github.com/koajs/csrf/blob/master/index.js#L70 https://github.com/koajs/csrf/blob/master/index.js#L119

Here you found the spec: http://curl.haxx.se/rfc/cookie_spec.html "This string is a sequence of characters excluding semi-colon, comma and white space."

When one would set this csrf-token as a cookie, then it would not work, right?

tj commented 10 years ago

cookie libs should escape but yeah there's no reason for us to use a semicolon there

jonathanong commented 10 years ago

the csrf token itself is never sent through cookies. the csrf secret this.session.secret is (though if you use a db store then it won't). that spec is outdated and not a real spec (afaik). the way it works should be fine.

remoe commented 10 years ago

@visionmedia, yes the libs 'should' escape this. But in koa it doesn't do this by default:

https://github.com/jed/cookies/blob/master/lib/cookies.js#L50 https://github.com/jed/cookies/blob/master/lib/cookies.js#L84

right? ;)

@jonathanong , yes your sample doesn't use cookies. but this should be an option. For example Angular.js use this by default:

http://docs.angularjs.org/api/ng.$http#description_security-considerations_cross-site-request-forgery-protection

And here is the real specification:

http://www.w3.org/Protocols/rfc2109/rfc2109 / Set-Cookie Syntax

jonathanong commented 10 years ago

you can open a PR in jed/cookies

you can't use this library for angular since this library doesn't send a csrf token as a cookie. you could just create another library specifically for angular.

jonathanong commented 10 years ago

or we could just use something else instead of a semicolon so you could. suggestion?

remoe commented 10 years ago

I think most of the current code will work, also with angular or whatever. One can set the cookie by self like:

this.cookies.set('csrf-token', this.csrf, { expires: expire, httpOnly: false });

So, the only thing is the semicolon. Escaping the semicolon should also work, but this is not optimal. This one:

http://www.senchalabs.org/connect/csrf.html

doesn't use a semicolon.

jonathanong commented 10 years ago

hmmm we've been setting JSON sessions with commas and probably whitespace. wonder how that has been working

jonathanong commented 10 years ago

https://github.com/koajs/csrf/commit/4cd36fb4b3b73f44751d7705f6dccfb69a8f50b8

remoe commented 10 years ago

Thanks, tested ;)