The reference implementation notably does not make length checks here, which can result in negative lengths in the calculation of $ciphertext, or negative offsets in the calculation of $mac:
Based on the implementation of Binary::safeSubstr when encountering negative offsets, this is ultimately identical to PHP's (i.e. counting backwards from the end of the string). This can lead to curious situations where (dependent on length) the bytes from the mac can be overlapped with the nonce, and a subset of the mac used as the ciphertext.
Aside from overlap (which in theory means that certain payloads are parsed identically even though they are different lengths), lengths of the interpreted ciphertext and mac are also fairly unintuitive here around certain boundaries (with lengths decreasing, and then increasing again). I don't think that this is abusable in practice, but it is probably an odd enough behaviour that it should be avoided?
Would a sensible behaviour in this example be to abort the decryption if $len <= self::NONCE_SIZE + self::MAC_SIZE? Obviously having a zero length ciphertext isn't likely to be legitimate, but we at least dispense of negative offsets here. I don't believe this early abort will have any security impact because the decision is made by examining the ciphertext and associated data alone, and is not interacting with the plaintext (e.g. so wouldn't be abusable in the same way as padding errors in a padding oracle attack). Still, I think it probably makes sense to formalise the early abort here as part of the spec?
For public modes, when calculating the signature offset based on the length subtraction from the end of the string, I think for similar reasons it would make sense to ensure that the signature offset is non-negative (i.e. that the payload is at least "signatureLength" long?)
For consistency, should the spec specify how to handle cases where a PASETO payload is received with an insufficient length?
Language is largely similar across the board, but using v4 decrypt as a reference, see step 4: https://github.com/paseto-standard/paseto-spec/blob/6143e466f8e69204d10ecc7e4e68e49a05b743b4/docs/01-Protocol-Versions/Version4.md#L73-L84
The reference implementation notably does not make length checks here, which can result in negative lengths in the calculation of
$ciphertext, or negative offsets in the calculation of$mac:https://github.com/paragonie/paseto/blob/6e3ce7fb5dfb2da512f38877e0b6e987ab519d8b/src/Protocol/Version4.php#L467-L474
Based on the implementation of
Binary::safeSubstrwhen encountering negative offsets, this is ultimately identical to PHP's (i.e. counting backwards from the end of the string). This can lead to curious situations where (dependent on length) the bytes from the mac can be overlapped with the nonce, and a subset of the mac used as the ciphertext.e.g.
Aside from overlap (which in theory means that certain payloads are parsed identically even though they are different lengths), lengths of the interpreted ciphertext and mac are also fairly unintuitive here around certain boundaries (with lengths decreasing, and then increasing again). I don't think that this is abusable in practice, but it is probably an odd enough behaviour that it should be avoided?
Would a sensible behaviour in this example be to abort the decryption if
$len <= self::NONCE_SIZE + self::MAC_SIZE? Obviously having a zero length ciphertext isn't likely to be legitimate, but we at least dispense of negative offsets here. I don't believe this early abort will have any security impact because the decision is made by examining the ciphertext and associated data alone, and is not interacting with the plaintext (e.g. so wouldn't be abusable in the same way as padding errors in a padding oracle attack). Still, I think it probably makes sense to formalise the early abort here as part of the spec?For public modes, when calculating the signature offset based on the length subtraction from the end of the string, I think for similar reasons it would make sense to ensure that the signature offset is non-negative (i.e. that the payload is at least "signatureLength" long?)