Bug 2566

This concerns part of the function ssh_packet_send2_wrapped() in the file packet.c.

The functionality of adding extra padding contains two integer overflows which can be triggered for certain values of extra_pad, block_size and len. Firstly, the computation roundup(state->extra_pad, block_size) can return 0 for certain values of extra_pad and block_size. This causes state->extra_pad = 0 but this variable is used subsequently in a modular operation. Secondly, the assignment padlen += pad might overflow for certain values of extra_pad, padlen and block_size. This can cause the length of the padding appended to the outgoing packet to be strictly less than 4; because no sanity check on the padding length follows the adding of extra padding. 

The computation roundup(state->extra_pad, block_size) rounds up extra padding to the nearest multiple of the block size. For e.g. block_size = 8, this computation will wrap to 0 when the requested extra padding length is larger or equal to 249. For larger block sizes the wrapping will start at a smaller value. Because the variable state->extra_pad is used subsequently in a modular operation, a floating point exception will be raised when the variable state->extra_pad is set to zero.

It is possible that a packet will be finalised with less than 4 bytes of padding, which is a violation of RFC 4253 section 6 that states: "There MUST be at least four bytes of padding". On the receiver end, a server/client will drop a packet (with a SSH_ERR_CONN_CORRUPT error) if the padding is less than 4. For e.g. a block size of 8 the padding appended to the outgoing packet will be less than 4 if 240 < extra_pad < 249 and len = 237, 238, 239, 240, 485, 486, 488, ....

I acknowledge the fact that extra padding is (atm) exclusively used for padding user passwords during authentication and used in a way that does not trigger the behaviour described above.