Skip to content

Commit

Permalink
Fix SCP server side
Browse files Browse the repository at this point in the history
SCP on the server side would get an EAGAIN around the 128KB mark, which
would trigger an error. That error in-turn would cause two attempts to
close the file, which would segfault.

Also fix inverted error return status on scpclient.
  • Loading branch information
LinuxJedi committed Feb 19, 2025
1 parent 697f54a commit a9c7ebc
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 3 deletions.
1 change: 1 addition & 0 deletions apps/wolfsshd/test/run_all_sshd_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ test_cases=(
"sshd_term_size_test.sh"
"sshd_large_sftp_test.sh"
"sshd_bad_sftp_test.sh"
"sshd_scp_fail.sh"
"sshd_term_close_test.sh"
"ssh_kex_algos.sh"
)
Expand Down
47 changes: 47 additions & 0 deletions apps/wolfsshd/test/sshd_scp_fail.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/bin/sh

# sshd local test

PWD=`pwd`
cd ../../..

TEST_SCP_CLIENT="./examples/scpclient/wolfscp"
USER=`whoami`
PRIVATE_KEY="./keys/hansel-key-ecc.der"
PUBLIC_KEY="./keys/hansel-key-ecc.pub"

if [ -z "$1" ] || [ -z "$2" ]; then
echo "expecting host and port as arguments"
echo "./sshd_exec_test.sh 127.0.0.1 22222"
exit 1
fi

mkdir test-$$

OUTDIR="`pwd`/test-$$"

dd if=/dev/random of=$OUTDIR/test.dat bs=1024 count=512

echo "$TEST_SCP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -S$OUTDIR/test.dat:. -H $1 -p $2"
$TEST_SCP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -S$OUTDIR/test.dat:. -H $1 -p $2

RESULT=$?
if [ "$RESULT" != "0" ]; then
echo "Expecting to pass transfer"
exit 1
fi

MD5SOURCE=`md5sum $OUTDIR/test.dat | awk '{ print $1 }'`
MD5DEST=`md5sum test.dat | awk '{ print $1 }'`

if [ "$MD5SOURCE" != "$MD5DEST" ]; then
echo "Files do not match $MD5SOURCE != $MD5DEST"
exit 1
fi

rm -rf test-$$
rm testout.dat

cd $PWD
exit 0

2 changes: 1 addition & 1 deletion examples/scpclient/scpclient.c
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ THREAD_RETURN WOLFSSH_THREAD scp_client(void* args)
wc_ecc_fp_free(); /* free per thread cache */
#endif

if (ret != WS_SUCCESS)
if ((ret != WS_SUCCESS) && (ret != WS_CHANNEL_CLOSED))
((func_args*)args)->return_code = 1;
return 0;
}
Expand Down
18 changes: 16 additions & 2 deletions src/wolfscp.c
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ int DoScpSource(WOLFSSH* ssh)
ssh->scpBufferedSz);
if (ret == WS_WINDOW_FULL || ret == WS_REKEYING) {
ret = wolfSSH_worker(ssh, NULL);
if (ret == WS_SUCCESS)
if (ret == WS_SUCCESS || ssh->error == WS_WANT_READ)
continue;
}
if (ret == WS_EXTDATA) {
Expand All @@ -616,8 +616,10 @@ int DoScpSource(WOLFSSH* ssh)
* open file descriptor before exit */
ScpSendCtx* sendCtx = NULL;
sendCtx = (ScpSendCtx*)wolfSSH_GetScpSendCtx(ssh);
if (sendCtx != NULL)
if (sendCtx != NULL) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
}
#endif
WLOG(WS_LOG_ERROR, scpError, "failed to send file", ret);
break;
Expand Down Expand Up @@ -1181,6 +1183,7 @@ static int ParseBasePathHelper(WOLFSSH* ssh, int cmdSz)

if (ScpPushDir(ssh->fs, &ctx, ssh->scpBasePath, ssh->ctx->heap) != WS_SUCCESS) {
WLOG(WS_LOG_DEBUG, "scp : issue opening base dir");
ssh->error = WS_INVALID_PATH_E;
ret = WS_FATAL_ERROR;
}
else {
Expand Down Expand Up @@ -2021,6 +2024,7 @@ int wsScpRecvCallback(WOLFSSH* ssh, int state, const char* basePath,
WLOG(WS_LOG_ERROR, scpError, "scp receive callback unable "
"to write requested size to file", bytes);
WFCLOSE(ssh->fs, fp);
fp = NULL;
ret = WS_SCP_ABORT;
} else {
#ifdef WOLFSCP_FLUSH
Expand All @@ -2047,6 +2051,7 @@ int wsScpRecvCallback(WOLFSSH* ssh, int state, const char* basePath,
flush_bytes = 0;
#endif
WFCLOSE(ssh->fs, fp);
fp = NULL;
}

/* set timestamp info */
Expand Down Expand Up @@ -2587,6 +2592,7 @@ static int ScpProcessEntry(WOLFSSH* ssh, char* fileName, word64* mTime,
if ((sendCtx->fp != NULL) &&
((ret < 0) || (*totalFileSz == (word32)ret))) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
}
}

Expand Down Expand Up @@ -2758,6 +2764,7 @@ int wsScpSendCallback(WOLFSSH* ssh, int state, const char* peerRequest,
if ((sendCtx != NULL) && (sendCtx->fp != NULL) &&
((ret < 0) || (*totalFileSz == (word32)ret))) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
}

break;
Expand Down Expand Up @@ -2840,13 +2847,20 @@ int wsScpSendCallback(WOLFSSH* ssh, int state, const char* peerRequest,
break;
}

if (sendCtx->fp == NULL) {
WLOG(WS_LOG_ERROR, "scp: file has been closed, abort");
ret = WS_SCP_ABORT;
break;
}

ret = (word32)WFREAD(ssh->fs, buf, 1, bufSz, sendCtx->fp);
if (ret == 0) { /* handle case of EOF */
ret = WS_EOF;
}

if ((ret <= 0) || (fileOffset + ret == *totalFileSz)) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
}

break;
Expand Down

0 comments on commit a9c7ebc

Please sign in to comment.