Ticket #244 (closed defect: fixed)

Opened 5 years ago

Last modified 23 months ago

scp_send may transmit not initialised memory

Reported by: torsten.rupp Owned by: mback2k
Priority: normal Milestone:
Component: SCP Version: 1.4.1
Keywords: scp_send Cc:
Blocked By: Blocks:

Description

In the function scp_send() the transmission function _libssh2_channel_process_startup() may be called with a message size larger than the actual initialised message buffer. This cause with e. g. valgrind a warning that not initialised memory is used.

From my analysis this is the problem:

  • at the beginning of then function scp_send() (line 787, scp.c) _libssh2_shell_quotedsize() is called to detect the memory space needed for session->scpRecv_command. This size is the _maximum_ size of the encoded message.

session->scpSend_command_len =

_libssh2_shell_quotedsize(path) + sizeof("scp -t ") +

((mtime
atime)?1:0);
  • the memory is allocated (line 791, scp.c)
  • the message content is formated with a snprintf()-call (line 799, scp.c)

snprintf((char *)session->scpSend_command, session->scpSend_command_len,

"scp -%st ", (mtime
atime)?"p":"");
  • the length of the resulting string is calculate (line 802, scp.c)

cmd_len = strlen((char *)session->scpSend_command);

  • the path parameter is added with possible quotes (line 804, scp.c)

(void)shell_quotearg(path,

&session->scpSend_command[cmd_len],
session->scpSend_command_len - cmd_len);

  • the command is terminated with NUL character, but not necessarily at the right place. The quoted path string may be shorter than the previously calculated max. length (line 808, scp.c)

session->scpSend_command[session->scpSend_command_len - 1] = '\0';

  • the formated message is send with the _maximum_ length, not the actual length (line 842, scp.c)

rc = _libssh2_channel_process_startup(session->scpSend_channel, "exec",

sizeof("exec") - 1,
(char *) session->scpSend_command,
session->scpSend_command_len);

This cause that the called transmission function access not initialised memory. This may not harm as long as the receiver will respect the terminating NUL character in the message. But at least more data then necessary is transmitted (path plus some random data) and checking tools like e. g. valgrind report a warning.

Change History

comment:1 Changed 5 years ago by bagder

  • Milestone 1.4.0 deleted

Milestone 1.4.0 deleted

comment:2 Changed 2 years ago by mback2k

  • Resolution set to fixed
  • Status changed from new to closed

Fixed with commit b99204f2896b0cdafa3ecc0736f0252ce44c32c7. Thanks.

comment:3 Changed 23 months ago by mback2k

  • Owner set to mback2k

In b99204f2896b0cdafa3ecc0736f0252ce44c32c7/libssh2:

scp.c: fix that scp_send may transmit not initialised memory

Fixes ticket 244. Thanks Torsten.

Note: See TracTickets for help on using tickets.