mirror of
https://git.libssh.org/projects/libssh.git
synced 2025-08-08 19:02:06 +03:00
packet: Do not decrypt zero length rest of buffer
If we receive a packet of length exactly blocksize, then packet_decrypt gets called on a buffer of size 0. The check at the beginning of packet_decrypt indicates that the function should be called on buffers of at least one blocksize, though the check allows through zero length. As is packet_decrypt can return -1 when len is 0 because malloc can return NULL in this case: according to the ISO C standard, malloc is free to return NULL or a pointer that can be freed when size == 0, and uclibc by default will return NULL here (in "non-glibc-compatible" mode). The net result is that when using uclibc connections with libssh can anomalously fail. Alternatively, packet_decrypt (and probably packet_encrypt for consistency) could be made to always succeed on len == 0 without depending on the behavior of malloc. Thanks to Josh Berlin for bringing conneciton failures with uclibc to my attention. Signed-off-by: Alan Dunn <amdunn@gmail.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This commit is contained in:
committed by
Andreas Schneider
parent
4ea4e12df2
commit
e7f831f0a3
@@ -152,7 +152,7 @@ int ssh_packet_socket_callback(const void *data, size_t receivedlen, void *user)
|
|||||||
const uint8_t *packet;
|
const uint8_t *packet;
|
||||||
int to_be_read;
|
int to_be_read;
|
||||||
int rc;
|
int rc;
|
||||||
uint32_t len, compsize, payloadsize;
|
uint32_t len, compsize, payloadsize, buffer_len;
|
||||||
uint8_t padding;
|
uint8_t padding;
|
||||||
size_t processed = 0; /* number of byte processed from the callback */
|
size_t processed = 0; /* number of byte processed from the callback */
|
||||||
|
|
||||||
@@ -251,13 +251,18 @@ int ssh_packet_socket_callback(const void *data, size_t receivedlen, void *user)
|
|||||||
* Decrypt the rest of the packet (blocksize bytes already
|
* Decrypt the rest of the packet (blocksize bytes already
|
||||||
* have been decrypted)
|
* have been decrypted)
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
/* The following check avoids decrypting zero bytes */
|
||||||
|
buffer_len = buffer_get_rest_len(session->in_buffer);
|
||||||
|
if (buffer_len != blocksize) {
|
||||||
rc = packet_decrypt(session,
|
rc = packet_decrypt(session,
|
||||||
((uint8_t*)buffer_get_rest(session->in_buffer) + blocksize),
|
((uint8_t*)buffer_get_rest(session->in_buffer) + blocksize),
|
||||||
buffer_get_rest_len(session->in_buffer) - blocksize);
|
buffer_len - blocksize);
|
||||||
if (rc < 0) {
|
if (rc < 0) {
|
||||||
ssh_set_error(session, SSH_FATAL, "Decrypt error");
|
ssh_set_error(session, SSH_FATAL, "Decrypt error");
|
||||||
goto error;
|
goto error;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/* copy the last part from the incoming buffer */
|
/* copy the last part from the incoming buffer */
|
||||||
packet = ((uint8_t *)data) + processed;
|
packet = ((uint8_t *)data) + processed;
|
||||||
|
@@ -106,7 +106,7 @@ int ssh_packet_socket_callback1(const void *data, size_t receivedlen, void *user
|
|||||||
size_t processed=0;
|
size_t processed=0;
|
||||||
uint32_t padding;
|
uint32_t padding;
|
||||||
uint32_t crc;
|
uint32_t crc;
|
||||||
uint32_t len;
|
uint32_t len, buffer_len;
|
||||||
ssh_session session=(ssh_session)user;
|
ssh_session session=(ssh_session)user;
|
||||||
|
|
||||||
switch (session->packet_state){
|
switch (session->packet_state){
|
||||||
@@ -168,13 +168,18 @@ int ssh_packet_socket_callback1(const void *data, size_t receivedlen, void *user
|
|||||||
* We decrypt everything, missing the lenght part (which was
|
* We decrypt everything, missing the lenght part (which was
|
||||||
* previously read, unencrypted, and is not part of the buffer
|
* previously read, unencrypted, and is not part of the buffer
|
||||||
*/
|
*/
|
||||||
if (packet_decrypt(session,
|
buffer_len = ssh_buffer_get_len(session->in_buffer);
|
||||||
|
if (buffer_len > 0) {
|
||||||
|
int rc;
|
||||||
|
rc = packet_decrypt(session,
|
||||||
ssh_buffer_get_begin(session->in_buffer),
|
ssh_buffer_get_begin(session->in_buffer),
|
||||||
ssh_buffer_get_len(session->in_buffer)) < 0) {
|
buffer_len);
|
||||||
|
if (rc < 0) {
|
||||||
ssh_set_error(session, SSH_FATAL, "Packet decrypt error");
|
ssh_set_error(session, SSH_FATAL, "Packet decrypt error");
|
||||||
goto error;
|
goto error;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
#ifdef DEBUG_CRYPTO
|
#ifdef DEBUG_CRYPTO
|
||||||
ssh_print_hexa("read packet decrypted:", ssh_buffer_get_begin(session->in_buffer),
|
ssh_print_hexa("read packet decrypted:", ssh_buffer_get_begin(session->in_buffer),
|
||||||
ssh_buffer_get_len(session->in_buffer));
|
ssh_buffer_get_len(session->in_buffer));
|
||||||
@@ -300,6 +305,8 @@ int packet_send1(ssh_session session) {
|
|||||||
ssh_buffer_get_len(session->out_buffer));
|
ssh_buffer_get_len(session->out_buffer));
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
/* session->out_buffer should have more than sizeof(uint32_t) bytes
|
||||||
|
in it as required for packet_encrypt */
|
||||||
packet_encrypt(session, (unsigned char *)ssh_buffer_get_begin(session->out_buffer) + sizeof(uint32_t),
|
packet_encrypt(session, (unsigned char *)ssh_buffer_get_begin(session->out_buffer) + sizeof(uint32_t),
|
||||||
ssh_buffer_get_len(session->out_buffer) - sizeof(uint32_t));
|
ssh_buffer_get_len(session->out_buffer) - sizeof(uint32_t));
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user