search

kataras / jwt

A fast and simple JWT implementation for Go
MIT License
204 stars 19 forks source link

Unnecessary custom base64 implementation #5

Open joonas-fi opened 3 years ago

joonas-fi commented 3 years ago

Go has RawStdEncoding which makes this unnecessary:

https://github.com/kataras/jwt/blob/1639fcff96f82f7ff118fcff6e1fbd0e01754f2c/token.go#L247

Not trying to nitpick here, rather I think security-wise it's dangerous as a concept to mutate untrusted input data before it's fed to a signature validation algorithm

joonas-fi commented 3 years ago

If you're open to more suggestions:

https://github.com/kataras/jwt/blob/1639fcff96f82f7ff118fcff6e1fbd0e01754f2c/util.go#L6

I think Go has recently done some optimizations where the compiler automatically optimizes these. I feel a bit unsafe with unsafe and doing special tricks in a security-critical library

kataras commented 2 years ago

Hello @joonas-fi,

BytesToString and Base64Encode are mostly helpers for users of this package. BytesToString is used just on a single return statement, inside the blockfile.go file, of course we can remove it from there if that's a "security" issue for you, I can live without it too. Base64Encode is used one "encodeToken" just to encode server-side data, the custom claims you/your program provides and NOT the user/client's one - I don't see any security-wise issue here but I am open for further discussion.