Does not properly encode/decode large values such as max uint64.

elliotcourant commented 8 months ago

I'm having an issue where large values are not being encoded properly. For example if I try to encode a value with a large uint64:

func TestBigUint64Claim(t *testing.T) {
    token := paseto.NewToken()
    var claims struct {
        ID uint64 `json:"id"`
    }
    claims.ID = math.MaxUint64 // 18446744073709551615
    require.NoError(t, token.Set("data", claims), "must be able to write a uint64 value to claims")
    token.SetExpiration(time.Now().Add(time.Minute))
    token.SetNotBefore(time.Now())
    token.SetIssuedAt(time.Now())

    secretKeyHex := "b4cbfb43df4ce210727d953e4a713307fa19bb7d9f85041438d9e11b942a37741eb9dbbbbc047c03fd70604e0071f0987e16b28b757225c11f00415d0e20b1a2"
    secretKey, _ := paseto.NewV4AsymmetricSecretKeyFromHex(secretKeyHex)

    fmt.Println(string(token.ClaimsJSON()))

    signed := token.V4Sign(secretKey, nil)

    parser := paseto.NewParser()
    publicKeyHex := "1eb9dbbbbc047c03fd70604e0071f0987e16b28b757225c11f00415d0e20b1a2"
    publicKey, _ := paseto.NewV4AsymmetricPublicKeyFromHex(publicKeyHex)

    parsedToken, err := parser.ParseV4Public(publicKey, signed, nil)
    require.NoError(t, err)
    inputJson := token.ClaimsJSON()
    parsedJson := parsedToken.ClaimsJSON()
    require.Equal(t, inputJson, parsedJson)
    var result struct {
        ID uint64 `json:"id"`
    }
    // Returns an error because the value of ID is 18446744073709552000
    require.NoError(t, parsedToken.Get("data", &result), "must decode data claims")
    require.Equal(t, claims.ID, result.ID, "ID should be equal")
}

The resulting token will look like this:

{
  "data": {
    "id": 18446744073709552000
  },
  "exp": "2023-11-11T17:12:19-06:00",
  "iat": "2023-11-11T17:11:19-06:00",
  "nbf": "2023-11-11T17:11:19-06:00"
}

This is because the values are encoded into JSON once when you call Set but then when you call V4Sign they are decoded in Claims() and then re-encoded in ClaimsJSON

This encode -> decode -> re-encode is causing the type of the data to be completely lost, and go has to assume the type; during the decode it assumes that the value is a float64 so you end up getting floating point errors and the value of the field changes during these transitions.

elliotcourant commented 8 months ago

A workaround for this is to add the string tag to fields with large numbers like:

  ID uint64 `json:"id,string"`

But I think that the value changing at all as part of the transformation process still qualifies this as a bug? Either limits should be imposed on what type of data can be provided to ensure that data does not change in the process of creating/parsing a token, or the data should simply not be transformed?

aidantwoods commented 8 months ago

Thanks for the report! I haven't had a chance to sit down and step through this in code myself, but I would agree that this looks like a bug.

The intermediary stored serialisations are a bit of a "hack" to offload marrying up the JSON with the requested type to the JSON unmarshaller (since doing it upfront means I'll get just plain strings,ints,etc... back instead of the wanted type).

My immediate thought here is that I could still immediately serialise the value within the token type when setting, but store it as a json.RawMessage instead so that it does not require the deserialise-serialise dance when serialising the whole token. The same would apply when decoding so that the decoding step is just delayed until the type is known.

elliotcourant commented 8 months ago

Thank you very much, hopefully its not a huge change. I had messed around with it a bit last night but didn't get very far. Thank you for the library though!

aidantwoods commented 8 months ago

https://github.com/aidantwoods/go-paseto/pull/48 should resolve this, I included your test to guard against regressing back to the current behaviour (thanks for that 🙂)

elliotcourant commented 8 months ago

Thank you very much!

aidantwoods commented 7 months ago

Fix available in v1.5.1. Thanks for the report!

aidantwoods / go-paseto

Does not properly encode/decode large values such as max uint64. #47

Set

Claims

ClaimsJSON