fix(postgres) use signed int for length prefix in PgCopyIn

[joeydewaal]

While looking into #3696 I noticed that CopyData uses an i32 to represent the length prefix. However, the AsyncRead impl uses an u32. https://github.com/launchbadge/sqlx/blob/a83395a360296db1251e6e4138b6e6331912c3d4/sqlx-postgres/src/copy.rs#L233

[abonander]

Yeah, ultimately this doesn't really matter because Postgres is hardcoded to reject messages above a certain size depending on the message type:

• CopyData uses PQ_LARGE_MESSAGE_LIMIT: https://github.com/postgres/postgres/blob/a5579a90af05814eb5dc2fd5f68ce803899d2504/src/backend/tcop/postgres.c#L435
• PQ_LARGE_MESSAGE_LIMIT is MaxAllocSize - 1: https://github.com/postgres/postgres/blob/master/src/include/libpq/libpq.h#L31
• MaxAllocSize is 1 GiB - 1: https://github.com/postgres/postgres/blob/a5579a90af05814eb5dc2fd5f68ce803899d2504/src/include/utils/memutils.h#L40

This includes the 4-byte length prefix, which is why it's actually 1 GiB - 5 for the message contents.

Writing a u32 instead of an i32 would just overflow to negative, which it also rejects. So while this is a tiny correctness improvement, it won't fix the bug.

The reason why #3440 is so dangerous is that unchecked casts from usize could overflow back around to a valid value, producing a buffer underflow attack.

I commented on #3696 (comment) for what the fix should look like.

[joeydewaal]

Writing a u32 instead of an i32 would just overflow to negative, which it also rejects. So while this is a tiny correctness improvement, it won't fix the bug.

I figured that's why postgres would still work with small payloads but would hang with big ones.

Thanks for the detailed reply, I'll try and come up with a more correct solution for this PR and #3696.