paseto-standard / paseto-spec

Specification for Platform Agnostic SEcurity TOkens (PASETO)
165 stars 9 forks source link

Fail when base64 data contains nonzero padding bits #28

Closed slashnick closed 2 years ago

slashnick commented 2 years ago

If the data in a base64 field of a PASETO token isn't a multiple of 6 bits, the last base64 character will be padded with 2 or 4 "zero" bits. But when decoding, most base64 libraries will ignore those last 4 bits.

eg. base64_encode(b"\xff") (11111111) == "_w" (111111 110000). But during decoding base64_decode("_w") (111111 110000) == base64_decode("_y") (111111 110001).

So given a valid PASETO token, you can construct a slightly different token with the same content but a different base64 representation. For example, these two public tokens both decode to the same content, despite having different final characters.

It seems weird for PASETO libraries to accept tokens that they clearly didn't generate.

slashnick commented 2 years ago

Oh, I guess a simpler way to construct duplicate tokens with would be to append ==. Maybe that should be blocked too?

paragonie-security commented 2 years ago

PASETO doesn't use Base64, PASETO uses Base64url (RFC 4648).

From the relevant section of the specification.

Nearly every component in a Paseto (except for the version, purpose, and the . separators) will be encoded using Base64url, without = padding.

If you'd like us to strengthen the wording ("MUST NOT include padding"; "decoders MUST be strict when decoding", etc.) then that's worth considering.

paragonie-security commented 2 years ago

https://github.com/paseto-standard/paseto-spec/commit/08be722de0bf5e38f39b6a06ffb2ee01479e6788

paragonie-security commented 2 years ago

Test vectors updated:

paragonie-security commented 2 years ago

PHP encoding library updated: https://github.com/paragonie/constant_time_encoding/releases/tag/v2.6.0

paragonie-security commented 2 years ago

Reference implementation updated: https://github.com/paragonie/paseto/releases/tag/v3.0.1

Thanks for reporting this.