broken sftp_quote logic
#2143

Comments

in the beginning of the function lib/ssh-libssh.c:sftp_quote() there's this line:

char *cmd = sshc->quote_item->data;

This is fine, because we know sshc->quote_item to be non-NULL due to earlier checks before this function is called.

This cmd pointer is then dereferenced and possibly increased but on line 2528 it is checked for NULL?

if(cmd) {

... if cmd would be NULL there, it would already have done wrong and crashed. It can't be NULL there!

But the if condition always ends with a return; and below, outside of the if block (on line 2657 in my version - this means this is code that can never be reached: dead code), there's a final check if sshc->quote_item is NULL (which again, it can't be) and if it is, it would call state()...

Edited 2 times

Dec 3, 2017

Dec 3, 2017

That function code is very similar with the SSH_SFTP_QUOTE: block in the ssh.c state machine. The same behavior seems to be there too. The if null check seems indeed incorrect. However, whether cmd is null at the call of this function, I cannot really tell. It depends on whether a quote_item can contain null data

Edited 1 time

Dec 4, 2017

I think the coverity errors 1424908, 1424902 and 1424901 are also shared between the two back-ends, though the last looks like a false negative. That is pretty much an argument for @kdudka 's suggestion to make the SCP/SFTP part separated from the back-end.

Figured out while reviwring code in the libssh backend. The pointer was
checked for NULL after having been dereferenced, so we know it would
always equal true or it would've crashed.
Pointed-out-by: Nikos Mavrogiannopoulos
Bug #2143

Figured out while reviewing code in the libssh backend. The pointer was
checked for NULL after having been dereferenced, so we know it would
always equal true or it would've crashed.
Pointed-out-by: Nikos Mavrogiannopoulos
Bug #2143Closes#2148