Skip to content

Commit 5dbf5d1

Browse files
ejohnstownejohnstown
committed
sftp: parse attrs and names via Get* parsers
Replace the hand-written ato32 plus localIdx + N > maxIdx checks in SFTP_ParseAttributes_buffer with GetUint32 and GetSkip, and drop the manual length plus copy sequences in wolfSSH_SFTP_DoStatus and wolfSSH_SFTP_DoName in favor of GetStringRef so the bounds-check arithmetic lives in one place.
1 parent 510a613 commit 5dbf5d1

1 file changed

Lines changed: 36 additions & 104 deletions

File tree

‎src/wolfsftp.c‎

Lines changed: 36 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -6010,68 +6010,45 @@ static int wolfSSH_SFTP_DoStatus(WOLFSSH* ssh, word32 reqId,
60106010
WS_SFTP_BUFFER* buffer)
60116011
{
60126012
word32 sz;
6013+
const byte* str;
60136014
word32 status = WOLFSSH_FTP_FAILURE;
60146015
word32 localIdx = wolfSSH_SFTP_buffer_idx(buffer);
60156016
word32 maxIdx = wolfSSH_SFTP_buffer_size(buffer);
60166017
byte* buf = wolfSSH_SFTP_buffer_data(buffer);
60176018

60186019
WOLFSSH_UNUSED(reqId);
6019-
if (localIdx + UINT32_SZ > maxIdx) {
6020+
if (GetUint32(&status, buf, maxIdx, &localIdx) != WS_SUCCESS) {
60206021
return WS_FATAL_ERROR;
60216022
}
6022-
ato32(buf + localIdx, &status);
6023-
localIdx += UINT32_SZ;
60246023

60256024
/* read error message */
6026-
if (localIdx + UINT32_SZ > maxIdx) {
6025+
if (GetStringRef(&sz, &str, buf, maxIdx, &localIdx) != WS_SUCCESS) {
60276026
return WS_FATAL_ERROR;
60286027
}
6029-
ato32(buf + localIdx, &sz);
6030-
localIdx += UINT32_SZ;
6031-
60326028
if (sz > 0) {
6033-
byte* s;
6034-
6035-
if (sz > maxIdx - localIdx) {
6036-
return WS_FATAL_ERROR;
6037-
}
6038-
s = (byte*)WMALLOC(sz + 1, ssh->ctx->heap, DYNTYPE_BUFFER);
6029+
byte* s = (byte*)WMALLOC(sz + 1, ssh->ctx->heap, DYNTYPE_BUFFER);
60396030
if (s == NULL) {
60406031
return WS_MEMORY_E;
60416032
}
6042-
6043-
/* make sure is null terminated string */
6044-
WMEMCPY(s, buf + localIdx, sz);
6033+
WMEMCPY(s, str, sz);
60456034
s[sz] = '\0';
60466035
WLOG(WS_LOG_SFTP, "Status Recv : %s", s);
60476036
WFREE(s, ssh->ctx->heap, DYNTYPE_BUFFER);
6048-
localIdx += sz;
60496037
}
60506038

60516039
/* read language tag */
6052-
if (localIdx + UINT32_SZ > maxIdx) {
6040+
if (GetStringRef(&sz, &str, buf, maxIdx, &localIdx) != WS_SUCCESS) {
60536041
return WS_FATAL_ERROR;
60546042
}
6055-
ato32(buf + localIdx, &sz);
6056-
localIdx += UINT32_SZ;
6057-
60586043
if (sz > 0) {
6059-
byte* s;
6060-
6061-
if (sz > maxIdx - localIdx) {
6062-
return WS_FATAL_ERROR;
6063-
}
6064-
s = (byte*)WMALLOC(sz + 1, ssh->ctx->heap, DYNTYPE_BUFFER);
6044+
byte* s = (byte*)WMALLOC(sz + 1, ssh->ctx->heap, DYNTYPE_BUFFER);
60656045
if (s == NULL) {
60666046
return WS_MEMORY_E;
60676047
}
6068-
6069-
/* make sure is null terminated string */
6070-
WMEMCPY(s, buf + localIdx, sz);
6048+
WMEMCPY(s, str, sz);
60716049
s[sz] = '\0';
60726050
WLOG(WS_LOG_SFTP, "Status Language : %s", s);
60736051
WFREE(s, ssh->ctx->heap, DYNTYPE_BUFFER);
6074-
localIdx += sz;
60756052
}
60766053

60776054
wolfSSH_SFTP_buffer_seek(buffer, 0, localIdx);
@@ -6132,86 +6109,62 @@ int SFTP_ParseAttributes_buffer(WOLFSSH* ssh, WS_SFTP_FILEATRB* atr, byte* buf,
61326109

61336110
WMEMSET(atr, 0, sizeof(WS_SFTP_FILEATRB));
61346111

6135-
if (localIdx + UINT32_SZ > maxIdx) {
6112+
/* get flags */
6113+
if (GetUint32(&atr->flags, buf, maxIdx, &localIdx) != WS_SUCCESS) {
61366114
return WS_BUFFER_E;
61376115
}
61386116

6139-
/* get flags */
6140-
ato32(buf + localIdx, &atr->flags); localIdx += UINT32_SZ;
6141-
61426117
/* check if size attribute present */
61436118
if (atr->flags & WOLFSSH_FILEATRB_SIZE) {
6144-
if (localIdx + (2*UINT32_SZ) > maxIdx) {
6119+
if (GetUint32(&atr->sz[1], buf, maxIdx, &localIdx) != WS_SUCCESS
6120+
|| GetUint32(&atr->sz[0], buf, maxIdx, &localIdx)
6121+
!= WS_SUCCESS) {
61456122
return WS_BUFFER_E;
61466123
}
6147-
ato32(buf + localIdx, &atr->sz[1]); localIdx += UINT32_SZ;
6148-
ato32(buf + localIdx, &atr->sz[0]); localIdx += UINT32_SZ;
61496124
}
61506125

61516126
/* check if uid and gid attribute present */
61526127
if (atr->flags & WOLFSSH_FILEATRB_UIDGID) {
6153-
if (localIdx + (2*UINT32_SZ) > maxIdx) {
6128+
if (GetUint32(&atr->uid, buf, maxIdx, &localIdx) != WS_SUCCESS
6129+
|| GetUint32(&atr->gid, buf, maxIdx, &localIdx)
6130+
!= WS_SUCCESS) {
61546131
return WS_BUFFER_E;
61556132
}
6156-
ato32(buf + localIdx, &atr->uid); localIdx += UINT32_SZ;
6157-
ato32(buf + localIdx, &atr->gid); localIdx += UINT32_SZ;
61586133
}
61596134

61606135
/* check if permissions attribute present */
61616136
if (atr->flags & WOLFSSH_FILEATRB_PERM) {
6162-
if (localIdx + UINT32_SZ > maxIdx) {
6137+
if (GetUint32(&atr->per, buf, maxIdx, &localIdx) != WS_SUCCESS) {
61636138
return WS_BUFFER_E;
61646139
}
6165-
ato32(buf + localIdx, &atr->per); localIdx += UINT32_SZ;
61666140
}
61676141

61686142
/* check if time attribute present */
61696143
if (atr->flags & WOLFSSH_FILEATRB_TIME) {
6170-
if (localIdx + (2*UINT32_SZ) > maxIdx) {
6144+
if (GetUint32(&atr->atime, buf, maxIdx, &localIdx) != WS_SUCCESS
6145+
|| GetUint32(&atr->mtime, buf, maxIdx, &localIdx)
6146+
!= WS_SUCCESS) {
61716147
return WS_BUFFER_E;
61726148
}
6173-
ato32(buf + localIdx, &atr->atime); localIdx += UINT32_SZ;
6174-
ato32(buf + localIdx, &atr->mtime); localIdx += UINT32_SZ;
61756149
}
61766150

61776151
/* check if extended attributes are present */
61786152
if (atr->flags & WOLFSSH_FILEATRB_EXT) {
61796153
word32 i;
6180-
word32 sz;
61816154

6182-
if (localIdx + UINT32_SZ > maxIdx) {
6155+
if (GetUint32(&atr->extCount, buf, maxIdx, &localIdx) != WS_SUCCESS) {
61836156
return WS_BUFFER_E;
61846157
}
6185-
ato32(buf + localIdx, &atr->extCount); localIdx += UINT32_SZ;
61866158

61876159
for (i = 0; i < atr->extCount; i++) {
6188-
/* @TODO in the process of storing attributes */
6189-
if (localIdx + UINT32_SZ > maxIdx) {
6160+
/* @TODO extension type, in the process of storing attributes */
6161+
if (GetSkip(buf, maxIdx, &localIdx) != WS_SUCCESS) {
61906162
return WS_BUFFER_E;
61916163
}
6192-
ato32(buf + localIdx, &sz); localIdx += UINT32_SZ;
6193-
6194-
if (sz > 0) {
6195-
if (localIdx + sz > maxIdx) {
6196-
return WS_BUFFER_E;
6197-
}
6198-
/* @TODO extension type */
6199-
localIdx += sz;
6200-
}
6201-
6202-
/* @TODO in the process of storing attributes */
6203-
if (localIdx + UINT32_SZ > maxIdx) {
6164+
/* @TODO extension data, in the process of storing attributes */
6165+
if (GetSkip(buf, maxIdx, &localIdx) != WS_SUCCESS) {
62046166
return WS_BUFFER_E;
62056167
}
6206-
ato32(buf + localIdx, &sz); localIdx += UINT32_SZ;
6207-
6208-
if (sz > 0) {
6209-
if (localIdx + sz > maxIdx) {
6210-
return WS_BUFFER_E;
6211-
}
6212-
/* @TODO extension data */
6213-
localIdx += sz;
6214-
}
62156168
}
62166169
}
62176170

@@ -6462,6 +6415,9 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh)
64626415

64636416
while (count > 0) {
64646417
word32 sz;
6418+
const byte* str;
6419+
byte* nameBuf = wolfSSH_SFTP_buffer_data(&state->buffer);
6420+
word32 nameMax = wolfSSH_SFTP_buffer_size(&state->buffer);
64656421
WS_SFTPNAME* tmp = wolfSSH_SFTPNAME_new(ssh->ctx->heap);
64666422

64676423
count--;
@@ -6478,8 +6434,9 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh)
64786434
n = tmp;
64796435

64806436
/* get filename size and name */
6481-
if (wolfSSH_SFTP_buffer_ato32(&state->buffer, &sz) !=
6482-
WS_SUCCESS) {
6437+
localIdx = wolfSSH_SFTP_buffer_idx(&state->buffer);
6438+
if (GetStringRef(&sz, &str, nameBuf, nameMax, &localIdx)
6439+
!= WS_SUCCESS) {
64836440
ret = WS_BUFFER_E;
64846441
break;
64856442
}
@@ -6491,24 +6448,13 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh)
64916448
ret = WS_MEMORY_E;
64926449
break;
64936450
}
6494-
6495-
if (wolfSSH_SFTP_buffer_idx(&state->buffer) + sz >
6496-
wolfSSH_SFTP_buffer_size(&state->buffer)) {
6497-
ret = WS_FATAL_ERROR;
6498-
break;
6499-
}
6500-
WMEMCPY(tmp->fName,
6501-
wolfSSH_SFTP_buffer_data(&state->buffer) +
6502-
wolfSSH_SFTP_buffer_idx(&state->buffer),
6503-
sz);
6504-
wolfSSH_SFTP_buffer_seek(&state->buffer,
6505-
wolfSSH_SFTP_buffer_idx(&state->buffer), sz);
6451+
WMEMCPY(tmp->fName, str, sz);
65066452
tmp->fName[sz] = '\0';
65076453
}
65086454

65096455
/* get longname size and name */
6510-
if (wolfSSH_SFTP_buffer_ato32(&state->buffer, &sz) !=
6511-
WS_SUCCESS) {
6456+
if (GetStringRef(&sz, &str, nameBuf, nameMax, &localIdx)
6457+
!= WS_SUCCESS) {
65126458
ret = WS_BUFFER_E;
65136459
break;
65146460
}
@@ -6520,27 +6466,13 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh)
65206466
ret = WS_MEMORY_E;
65216467
break;
65226468
}
6523-
6524-
if (wolfSSH_SFTP_buffer_idx(&state->buffer) + sz >
6525-
wolfSSH_SFTP_buffer_size(&state->buffer)) {
6526-
ret = WS_FATAL_ERROR;
6527-
break;
6528-
}
6529-
WMEMCPY(tmp->lName,
6530-
wolfSSH_SFTP_buffer_data(&state->buffer) +
6531-
wolfSSH_SFTP_buffer_idx(&state->buffer),
6532-
sz);
6533-
wolfSSH_SFTP_buffer_seek(&state->buffer,
6534-
wolfSSH_SFTP_buffer_idx(&state->buffer), sz);
6469+
WMEMCPY(tmp->lName, str, sz);
65356470
tmp->lName[sz] = '\0';
65366471
}
65376472

65386473
/* get attributes */
6539-
localIdx = wolfSSH_SFTP_buffer_idx(&state->buffer);
65406474
ret = SFTP_ParseAttributes_buffer(ssh, &tmp->atrb,
6541-
wolfSSH_SFTP_buffer_data(&state->buffer),
6542-
&localIdx,
6543-
wolfSSH_SFTP_buffer_size(&state->buffer));
6475+
nameBuf, &localIdx, nameMax);
65446476
wolfSSH_SFTP_buffer_seek(&state->buffer, 0, localIdx);
65456477
if (ret != WS_SUCCESS) {
65466478
break;

0 commit comments

Comments
 (0)