Skip to content

Commit 1644bb7

Browse files
yosuke-wolfsslejohnstown
authored andcommitted
Accept exact-fit ScpBuffer in no-filesystem SCP send callback
1 parent 497da8a commit 1644bb7

3 files changed

Lines changed: 286 additions & 7 deletions

File tree

.github/workflows/os-check.yml

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,48 @@ jobs:
9797
path: wolfssh
9898
configure: ${{ matrix.config }} LDFLAGS="-L${{ github.workspace }}/build-dir/lib" CPPFLAGS="-I${{ github.workspace }}/build-dir/include"
9999
check: true
100+
101+
build_wolfssh_no_filesystem:
102+
needs:
103+
- build_wolfssl
104+
- create_matrix
105+
strategy:
106+
fail-fast: false
107+
matrix:
108+
wolfssl: ${{ fromJson(needs.create_matrix.outputs['versions']) }}
109+
name: Build and test wolfssh with NO_FILESYSTEM
110+
runs-on: ubuntu-latest
111+
timeout-minutes: 6
112+
steps:
113+
- name: Checking cache for wolfssl
114+
uses: actions/cache@v5
115+
with:
116+
path: build-dir/
117+
key: wolfssh-os-check-wolfssl-${{ matrix.wolfssl }}-ubuntu-latest
118+
fail-on-cache-miss: true
119+
120+
- uses: actions/checkout@v6
121+
with:
122+
path: wolfssh/
123+
124+
- name: autogen
125+
working-directory: ./wolfssh/
126+
run: ./autogen.sh
127+
128+
- name: configure
129+
working-directory: ./wolfssh/
130+
run: |
131+
./configure --enable-scp LDFLAGS="-L${{ github.workspace }}/build-dir/lib" CPPFLAGS="-I${{ github.workspace }}/build-dir/include -DNO_FILESYSTEM"
132+
133+
# The examples do not build without a filesystem, so build the
134+
# library and the unit test directly instead of running make all
135+
- name: make unit test
136+
working-directory: ./wolfssh/
137+
run: make tests/unit.test
138+
139+
- name: run unit test
140+
working-directory: ./wolfssh/
141+
run: |
142+
set -o pipefail
143+
LD_LIBRARY_PATH=${{ github.workspace }}/build-dir/lib ./tests/unit.test | tee unit-test.log
144+
grep "ScpSendCallback_ExactFitBuffer: SUCCESS" unit-test.log

src/wolfscp.c

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3716,10 +3716,12 @@ int wsScpSendCallback(WOLFSSH* ssh, int state, const char* peerRequest,
37163716
*aTime = sendBuffer->mTime;
37173717
*fileMode = sendBuffer->mode;
37183718

3719-
/* copy over buffer info */
3719+
/* copy over buffer info, idx past fileSz wraps the unsigned
3720+
* math and is caught by the guard */
37203721
ret = (bufSz < (sendBuffer->fileSz - sendBuffer->idx))?
37213722
bufSz : sendBuffer->fileSz - sendBuffer->idx;
3722-
if (sendBuffer->idx + ret >= sendBuffer->bufferSz) {
3723+
if (sendBuffer->idx > sendBuffer->fileSz ||
3724+
sendBuffer->idx + ret > sendBuffer->bufferSz) {
37233725
WLOG(WS_LOG_ERROR, scpState,
37243726
"potential buffer overflow caught, abort");
37253727
ret = WS_SCP_ABORT;
@@ -3739,17 +3741,19 @@ int wsScpSendCallback(WOLFSSH* ssh, int state, const char* peerRequest,
37393741
break;
37403742

37413743
case WOLFSSH_SCP_CONTINUE_FILE_TRANSFER:
3742-
/* copy over buffer info */
3743-
if (sendBuffer->idx >= sendBuffer->bufferSz) {
3744+
/* copy over buffer info, idx past fileSz would underflow the
3745+
* size math */
3746+
if (sendBuffer->idx > sendBuffer->bufferSz ||
3747+
sendBuffer->idx > sendBuffer->fileSz) {
37443748
WLOG(WS_LOG_ERROR, scpState,
3745-
"sendbuffer idx greater than buffer size, abort");
3749+
"sendbuffer idx out of range, abort");
37463750
ret = WS_SCP_ABORT;
37473751
break;
37483752
}
37493753
ret = (bufSz < (sendBuffer->fileSz - sendBuffer->idx))?
37503754
bufSz : sendBuffer->fileSz - sendBuffer->idx;
37513755
if (ret > 0) {
3752-
if (sendBuffer->idx + ret >= sendBuffer->bufferSz) {
3756+
if (sendBuffer->idx + ret > sendBuffer->bufferSz) {
37533757
ret = WS_SCP_ABORT;
37543758
WLOG(WS_LOG_ERROR, scpState, "buffer size issue, abort");
37553759
break;
@@ -3761,7 +3765,8 @@ int wsScpSendCallback(WOLFSSH* ssh, int state, const char* peerRequest,
37613765
ret = WS_EOF;
37623766
}
37633767

3764-
if (sendBuffer->status(ssh, sendBuffer->name,
3768+
if (sendBuffer->status != NULL &&
3769+
sendBuffer->status(ssh, sendBuffer->name,
37653770
WOLFSSH_SCP_CONTINUE_FILE_TRANSFER, sendBuffer)
37663771
!= WS_SUCCESS) {
37673772
WLOG(WS_LOG_DEBUG, scpState, "continue status fail, abort");

tests/unit.c

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14256,6 +14256,227 @@ static int test_ScpTimestamp_NoFollow(void)
1425614256
#endif /* WOLFSSH_SCP recv callback depth guard test */
1425714257

1425814258

14259+
#if defined(WOLFSSH_SCP) && !defined(WOLFSSH_SCP_USER_CALLBACKS) && \
14260+
defined(NO_FILESYSTEM)
14261+
14262+
static int scpSendTestStatus(WOLFSSH* ssh, const char* fileName,
14263+
enum WS_ScpFileStates state, ScpBuffer* file)
14264+
{
14265+
WOLFSSH_UNUSED(ssh);
14266+
WOLFSSH_UNUSED(fileName);
14267+
WOLFSSH_UNUSED(state);
14268+
WOLFSSH_UNUSED(file);
14269+
return WS_SUCCESS;
14270+
}
14271+
14272+
static int scpSendTestStatusFail(WOLFSSH* ssh, const char* fileName,
14273+
enum WS_ScpFileStates state, ScpBuffer* file)
14274+
{
14275+
WOLFSSH_UNUSED(ssh);
14276+
WOLFSSH_UNUSED(fileName);
14277+
WOLFSSH_UNUSED(state);
14278+
WOLFSSH_UNUSED(file);
14279+
return WS_FATAL_ERROR;
14280+
}
14281+
14282+
/* The default no-filesystem send callback must accept an ScpBuffer whose
14283+
* bufferSz exactly equals fileSz, in both single- and multi-chunk transfers,
14284+
* and must still refuse a fileSz larger than the buffer. */
14285+
static int test_ScpSendCallback_ExactFitBuffer(void)
14286+
{
14287+
WOLFSSH_CTX* ctx = NULL;
14288+
WOLFSSH* ssh = NULL;
14289+
ScpBuffer sendBuf;
14290+
byte data[16];
14291+
byte out[64];
14292+
char fileName[DEFAULT_SCP_FILE_NAME_SZ];
14293+
word64 mTime = 0;
14294+
word64 aTime = 0;
14295+
word32 totalSz = 0;
14296+
int fileMode = 0;
14297+
int result = 0;
14298+
int ret;
14299+
word32 i;
14300+
14301+
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
14302+
if (ctx == NULL)
14303+
return -1500;
14304+
ssh = wolfSSH_new(ctx);
14305+
if (ssh == NULL) {
14306+
wolfSSH_CTX_free(ctx);
14307+
return -1501;
14308+
}
14309+
14310+
for (i = 0; i < (word32)sizeof(data); i++)
14311+
data[i] = (byte)i;
14312+
14313+
WMEMSET(&sendBuf, 0, sizeof(sendBuf));
14314+
WMEMCPY(sendBuf.name, "file.txt", WSTRLEN("file.txt") + 1);
14315+
sendBuf.nameSz = (word32)WSTRLEN(sendBuf.name);
14316+
sendBuf.buffer = data;
14317+
sendBuf.bufferSz = (word32)sizeof(data);
14318+
sendBuf.fileSz = (word32)sizeof(data);
14319+
sendBuf.status = scpSendTestStatus;
14320+
WMEMSET(fileName, 0, sizeof(fileName));
14321+
14322+
/* exact fit, single chunk: the whole file is returned, not aborted */
14323+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_SINGLE_FILE_REQUEST, "file.txt",
14324+
fileName, (word32)sizeof(fileName), &mTime, &aTime, &fileMode, 0,
14325+
&totalSz, out, (word32)sizeof(out), &sendBuf);
14326+
if (ret != (int)sendBuf.fileSz)
14327+
result = -1502;
14328+
if (result == 0 && sendBuf.idx != sendBuf.fileSz)
14329+
result = -1503;
14330+
if (result == 0 && totalSz != sendBuf.fileSz)
14331+
result = -1504;
14332+
if (result == 0 && WMEMCMP(out, data, sizeof(data)) != 0)
14333+
result = -1505;
14334+
14335+
/* exact fit, multi-chunk: final chunk lands on the buffer boundary */
14336+
if (result == 0) {
14337+
sendBuf.idx = 0;
14338+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_SINGLE_FILE_REQUEST,
14339+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14340+
&fileMode, 0, &totalSz, out, 10, &sendBuf);
14341+
if (ret != 10)
14342+
result = -1506;
14343+
}
14344+
if (result == 0) {
14345+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_CONTINUE_FILE_TRANSFER,
14346+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14347+
&fileMode, 10, &totalSz, out, 10, &sendBuf);
14348+
if (ret != 6)
14349+
result = -1507;
14350+
if (result == 0 && WMEMCMP(out, data + 10, 6) != 0)
14351+
result = -1508;
14352+
}
14353+
if (result == 0) {
14354+
/* transfer complete: a further continue reports EOF, not abort */
14355+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_CONTINUE_FILE_TRANSFER,
14356+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14357+
&fileMode, 16, &totalSz, out, 10, &sendBuf);
14358+
if (ret != WS_EOF)
14359+
result = -1509;
14360+
}
14361+
14362+
/* fileSz larger than the buffer is still refused before any copy */
14363+
if (result == 0) {
14364+
sendBuf.idx = 0;
14365+
sendBuf.fileSz = sendBuf.bufferSz + 1;
14366+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_SINGLE_FILE_REQUEST,
14367+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14368+
&fileMode, 0, &totalSz, out, (word32)sizeof(out), &sendBuf);
14369+
if (ret != WS_SCP_ABORT)
14370+
result = -1510;
14371+
sendBuf.fileSz = sendBuf.bufferSz;
14372+
}
14373+
14374+
/* file smaller than the buffer keeps working */
14375+
if (result == 0) {
14376+
sendBuf.idx = 0;
14377+
sendBuf.fileSz = 8;
14378+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_SINGLE_FILE_REQUEST,
14379+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14380+
&fileMode, 0, &totalSz, out, (word32)sizeof(out), &sendBuf);
14381+
if (ret != 8)
14382+
result = -1511;
14383+
sendBuf.fileSz = sendBuf.bufferSz;
14384+
}
14385+
14386+
/* a NULL status callback must not crash the continue state */
14387+
if (result == 0) {
14388+
sendBuf.idx = 0;
14389+
sendBuf.status = NULL;
14390+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_SINGLE_FILE_REQUEST,
14391+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14392+
&fileMode, 0, &totalSz, out, 10, &sendBuf);
14393+
if (ret != 10)
14394+
result = -1512;
14395+
if (result == 0) {
14396+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_CONTINUE_FILE_TRANSFER,
14397+
"file.txt", fileName, (word32)sizeof(fileName), &mTime,
14398+
&aTime, &fileMode, 10, &totalSz, out, 10, &sendBuf);
14399+
if (ret != 6)
14400+
result = -1513;
14401+
}
14402+
sendBuf.status = scpSendTestStatus;
14403+
}
14404+
14405+
/* idx one past the buffer end must abort, not report EOF; idx == fileSz
14406+
* keeps the inner guard out of play so only the top guard can reject */
14407+
if (result == 0) {
14408+
sendBuf.idx = sendBuf.bufferSz + 1;
14409+
sendBuf.fileSz = sendBuf.bufferSz + 1;
14410+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_CONTINUE_FILE_TRANSFER,
14411+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14412+
&fileMode, 0, &totalSz, out, 10, &sendBuf);
14413+
if (ret != WS_SCP_ABORT)
14414+
result = -1514;
14415+
sendBuf.idx = 0;
14416+
sendBuf.fileSz = sendBuf.bufferSz;
14417+
}
14418+
14419+
/* a chunk that would run past the buffer end mid-continue must abort */
14420+
if (result == 0) {
14421+
sendBuf.idx = 10;
14422+
sendBuf.fileSz = sendBuf.bufferSz + 4;
14423+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_CONTINUE_FILE_TRANSFER,
14424+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14425+
&fileMode, 10, &totalSz, out, 10, &sendBuf);
14426+
if (ret != WS_SCP_ABORT)
14427+
result = -1515;
14428+
sendBuf.idx = 0;
14429+
sendBuf.fileSz = sendBuf.bufferSz;
14430+
}
14431+
14432+
/* idx past fileSz mid-continue must abort; idx + bufSz stays inside
14433+
* bufferSz here, so only the underflow guard can reject */
14434+
if (result == 0) {
14435+
sendBuf.idx = 5;
14436+
sendBuf.fileSz = 4;
14437+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_CONTINUE_FILE_TRANSFER,
14438+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14439+
&fileMode, 0, &totalSz, out, 10, &sendBuf);
14440+
if (ret != WS_SCP_ABORT)
14441+
result = -1516;
14442+
sendBuf.idx = 0;
14443+
sendBuf.fileSz = sendBuf.bufferSz;
14444+
}
14445+
14446+
/* same underflow shape in the single file request state */
14447+
if (result == 0) {
14448+
sendBuf.idx = 5;
14449+
sendBuf.fileSz = 4;
14450+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_SINGLE_FILE_REQUEST,
14451+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14452+
&fileMode, 0, &totalSz, out, 10, &sendBuf);
14453+
if (ret != WS_SCP_ABORT)
14454+
result = -1517;
14455+
sendBuf.idx = 0;
14456+
sendBuf.fileSz = sendBuf.bufferSz;
14457+
}
14458+
14459+
/* a failing status callback aborts the continue state even after a
14460+
* successful copy */
14461+
if (result == 0) {
14462+
sendBuf.idx = 0;
14463+
sendBuf.status = scpSendTestStatusFail;
14464+
ret = wsScpSendCallback(ssh, WOLFSSH_SCP_CONTINUE_FILE_TRANSFER,
14465+
"file.txt", fileName, (word32)sizeof(fileName), &mTime, &aTime,
14466+
&fileMode, 0, &totalSz, out, 10, &sendBuf);
14467+
if (ret != WS_SCP_ABORT)
14468+
result = -1518;
14469+
sendBuf.idx = 0;
14470+
sendBuf.status = scpSendTestStatus;
14471+
}
14472+
14473+
wolfSSH_free(ssh);
14474+
wolfSSH_CTX_free(ctx);
14475+
return result;
14476+
}
14477+
#endif /* WOLFSSH_SCP && !WOLFSSH_SCP_USER_CALLBACKS && NO_FILESYSTEM */
14478+
14479+
1425914480
/* ParseECCPubKey() Unit Test */
1426014481

1426114482
#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256
@@ -16254,6 +16475,14 @@ int wolfSSH_UnitTest(int argc, char** argv)
1625416475
testResult = testResult || unitResult;
1625516476
#endif
1625616477

16478+
#if defined(WOLFSSH_SCP) && !defined(WOLFSSH_SCP_USER_CALLBACKS) && \
16479+
defined(NO_FILESYSTEM)
16480+
unitResult = test_ScpSendCallback_ExactFitBuffer();
16481+
printf("ScpSendCallback_ExactFitBuffer: %s\n",
16482+
(unitResult == 0 ? "SUCCESS" : "FAILED"));
16483+
testResult = testResult || unitResult;
16484+
#endif
16485+
1625716486
#ifdef WOLFSSH_TEST_CAPTURING_ALLOCATOR
1625816487
unitResult = test_SshResourceFree_zeroesSecrets();
1625916488
printf("SshResourceFree_zeroesSecrets: %s\n",

0 commit comments

Comments
 (0)