devinc
commented
3 years ago I agree that this behavior should change. The docs do explain the behavior (and it is of course used correctly internally), but I did a bit more searching and have found many projects that do not properly check the return value. Several of those projects I looked at were using it with some kind of session identifier that in some cases had pretty limited entropy. This is an auth bypass for those projects.
I also believe that having it return false when passed an unsigned cookie shouldn't break too much existing code.
cookie-parser's signedCookie function has the following behavior when it encounters an unsigned value:
"If the value was not signed, the original value is returned."
This is subtle behavior, and it seems unlikely that a caller would actually know to check that the return value was different from what was passed in. If the caller depends on the signature mechanism to prevent tampering this could be a serious problem.
A cursory check shows all 3 callers on github are not checking the return value:
https://github.com/search?q=%22cookieparser.signedCookie%22+-path%3AcookieParser&type=Code&ref=advsearch&l=&l=
I'd suggest changing the API to return false if passed a non-signature cookie value, similar to failing the signature check.