u32 overflow in SignedPayload::from_payload

[C0x41lch0x41]

What version are you using?

Most up-to-date codebase

What did you do?

Called stellar_strkey::ed25519::SignedPayload::from_payload with following payload:

let payload: Vec<u8> = vec![
        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
        0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    ];

What did you expect to see?

Program exit with error DecodeError::Invalid

What did you see instead?

Program panics:

thread 'test_signed_payload_from_payload' panicked at src/ed25519.rs:262:12:
attempt to add with overflow
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/std/src/panicking.rs:595:5
   1: core::panicking::panic_fmt
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:67:14
   2: core::panicking::panic
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:117:5
   3: stellar_strkey::ed25519::SignedPayload::from_payload
             at ./src/ed25519.rs:262:12
   4: tests::test_signed_payload_from_payload
             at ./tests/tests.rs:249:16
   5: tests::test_signed_payload_from_payload::{{closure}}
             at ./tests/tests.rs:243:39
   6: core::ops::function::FnOnce::call_once
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/ops/function.rs:250:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/ops/function.rs:250:5

This is because of the following line:

inner_payload_len + (4 - inner_payload_len % 4) % 4

inner_payload_len is 0xffffffff so (4 - inner_payload_len % 4) % 4 = 1 so

inner_payload_len + (4 - inner_payload_len % 4) % 4 = u32::MAX + 1

which overflow.

[leighmcculloch]

We should make the inner_payload_len a u64, since the length of the payload can technically be up to u32::MAX, and so the value held by this variable needs to be able to hold a value greater than that.

However, in practice payloads supported by stellar-core are limited in size to either 32bytes or 64 bytes (I don't remember). So it would be reasonable I think to limit strkeys in this lib to that shorter length too.