Skip to content

Commit 6794601

Browse files
ejohnstownpadelsbach
ejohnstown
authored and
padelsbach
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 a79abf7 commit 6794601

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
@@ -6009,68 +6009,45 @@ static int wolfSSH_SFTP_DoStatus(WOLFSSH* ssh, word32 reqId,
60096009
WS_SFTP_BUFFER* buffer)
60106010
{
60116011
word32 sz;
6012+
const byte* str;
60126013
word32 status = WOLFSSH_FTP_FAILURE;
60136014
word32 localIdx = wolfSSH_SFTP_buffer_idx(buffer);
60146015
word32 maxIdx = wolfSSH_SFTP_buffer_size(buffer);
60156016
byte* buf = wolfSSH_SFTP_buffer_data(buffer);
60166017

60176018
WOLFSSH_UNUSED(reqId);
6018-
if (localIdx + UINT32_SZ > maxIdx) {
6019+
if (GetUint32(&status, buf, maxIdx, &localIdx) != WS_SUCCESS) {
60196020
return WS_FATAL_ERROR;
60206021
}
6021-
ato32(buf + localIdx, &status);
6022-
localIdx += UINT32_SZ;
60236022

60246023
/* read error message */
6025-
if (localIdx + UINT32_SZ > maxIdx) {
6024+
if (GetStringRef(&sz, &str, buf, maxIdx, &localIdx) != WS_SUCCESS) {
60266025
return WS_FATAL_ERROR;
60276026
}
6028-
ato32(buf + localIdx, &sz);
6029-
localIdx += UINT32_SZ;
6030-
60316027
if (sz > 0) {
6032-
byte* s;
6033-
6034-
if (sz > maxIdx - localIdx) {
6035-
return WS_FATAL_ERROR;
6036-
}
6037-
s = (byte*)WMALLOC(sz + 1, ssh->ctx->heap, DYNTYPE_BUFFER);
6028+
byte* s = (byte*)WMALLOC(sz + 1, ssh->ctx->heap, DYNTYPE_BUFFER);
60386029
if (s == NULL) {
60396030
return WS_MEMORY_E;
60406031
}
6041-
6042-
/* make sure is null terminated string */
6043-
WMEMCPY(s, buf + localIdx, sz);
6032+
WMEMCPY(s, str, sz);
60446033
s[sz] = '\0';
60456034
WLOG(WS_LOG_SFTP, "Status Recv : %s", s);
60466035
WFREE(s, ssh->ctx->heap, DYNTYPE_BUFFER);
6047-
localIdx += sz;
60486036
}
60496037

60506038
/* read language tag */
6051-
if (localIdx + UINT32_SZ > maxIdx) {
6039+
if (GetStringRef(&sz, &str, buf, maxIdx, &localIdx) != WS_SUCCESS) {
60526040
return WS_FATAL_ERROR;
60536041
}
6054-
ato32(buf + localIdx, &sz);
6055-
localIdx += UINT32_SZ;
6056-
60576042
if (sz > 0) {
6058-
byte* s;
6059-
6060-
if (sz > maxIdx - localIdx) {
6061-
return WS_FATAL_ERROR;
6062-
}
6063-
s = (byte*)WMALLOC(sz + 1, ssh->ctx->heap, DYNTYPE_BUFFER);
6043+
byte* s = (byte*)WMALLOC(sz + 1, ssh->ctx->heap, DYNTYPE_BUFFER);
60646044
if (s == NULL) {
60656045
return WS_MEMORY_E;
60666046
}
6067-
6068-
/* make sure is null terminated string */
6069-
WMEMCPY(s, buf + localIdx, sz);
6047+
WMEMCPY(s, str, sz);
60706048
s[sz] = '\0';
60716049
WLOG(WS_LOG_SFTP, "Status Language : %s", s);
60726050
WFREE(s, ssh->ctx->heap, DYNTYPE_BUFFER);
6073-
localIdx += sz;
60746051
}
60756052

60766053
wolfSSH_SFTP_buffer_seek(buffer, 0, localIdx);
@@ -6131,86 +6108,62 @@ int SFTP_ParseAttributes_buffer(WOLFSSH* ssh, WS_SFTP_FILEATRB* atr, byte* buf,
61316108

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

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

6138-
/* get flags */
6139-
ato32(buf + localIdx, &atr->flags); localIdx += UINT32_SZ;
6140-
61416116
/* check if size attribute present */
61426117
if (atr->flags & WOLFSSH_FILEATRB_SIZE) {
6143-
if (localIdx + (2*UINT32_SZ) > maxIdx) {
6118+
if (GetUint32(&atr->sz[1], buf, maxIdx, &localIdx) != WS_SUCCESS
6119+
|| GetUint32(&atr->sz[0], buf, maxIdx, &localIdx)
6120+
!= WS_SUCCESS) {
61446121
return WS_BUFFER_E;
61456122
}
6146-
ato32(buf + localIdx, &atr->sz[1]); localIdx += UINT32_SZ;
6147-
ato32(buf + localIdx, &atr->sz[0]); localIdx += UINT32_SZ;
61486123
}
61496124

61506125
/* check if uid and gid attribute present */
61516126
if (atr->flags & WOLFSSH_FILEATRB_UIDGID) {
6152-
if (localIdx + (2*UINT32_SZ) > maxIdx) {
6127+
if (GetUint32(&atr->uid, buf, maxIdx, &localIdx) != WS_SUCCESS
6128+
|| GetUint32(&atr->gid, buf, maxIdx, &localIdx)
6129+
!= WS_SUCCESS) {
61536130
return WS_BUFFER_E;
61546131
}
6155-
ato32(buf + localIdx, &atr->uid); localIdx += UINT32_SZ;
6156-
ato32(buf + localIdx, &atr->gid); localIdx += UINT32_SZ;
61576132
}
61586133

61596134
/* check if permissions attribute present */
61606135
if (atr->flags & WOLFSSH_FILEATRB_PERM) {
6161-
if (localIdx + UINT32_SZ > maxIdx) {
6136+
if (GetUint32(&atr->per, buf, maxIdx, &localIdx) != WS_SUCCESS) {
61626137
return WS_BUFFER_E;
61636138
}
6164-
ato32(buf + localIdx, &atr->per); localIdx += UINT32_SZ;
61656139
}
61666140

61676141
/* check if time attribute present */
61686142
if (atr->flags & WOLFSSH_FILEATRB_TIME) {
6169-
if (localIdx + (2*UINT32_SZ) > maxIdx) {
6143+
if (GetUint32(&atr->atime, buf, maxIdx, &localIdx) != WS_SUCCESS
6144+
|| GetUint32(&atr->mtime, buf, maxIdx, &localIdx)
6145+
!= WS_SUCCESS) {
61706146
return WS_BUFFER_E;
61716147
}
6172-
ato32(buf + localIdx, &atr->atime); localIdx += UINT32_SZ;
6173-
ato32(buf + localIdx, &atr->mtime); localIdx += UINT32_SZ;
61746148
}
61756149

61766150
/* check if extended attributes are present */
61776151
if (atr->flags & WOLFSSH_FILEATRB_EXT) {
61786152
word32 i;
6179-
word32 sz;
61806153

6181-
if (localIdx + UINT32_SZ > maxIdx) {
6154+
if (GetUint32(&atr->extCount, buf, maxIdx, &localIdx) != WS_SUCCESS) {
61826155
return WS_BUFFER_E;
61836156
}
6184-
ato32(buf + localIdx, &atr->extCount); localIdx += UINT32_SZ;
61856157

61866158
for (i = 0; i < atr->extCount; i++) {
6187-
/* @TODO in the process of storing attributes */
6188-
if (localIdx + UINT32_SZ > maxIdx) {
6159+
/* @TODO extension type, in the process of storing attributes */
6160+
if (GetSkip(buf, maxIdx, &localIdx) != WS_SUCCESS) {
61896161
return WS_BUFFER_E;
61906162
}
6191-
ato32(buf + localIdx, &sz); localIdx += UINT32_SZ;
6192-
6193-
if (sz > 0) {
6194-
if (localIdx + sz > maxIdx) {
6195-
return WS_BUFFER_E;
6196-
}
6197-
/* @TODO extension type */
6198-
localIdx += sz;
6199-
}
6200-
6201-
/* @TODO in the process of storing attributes */
6202-
if (localIdx + UINT32_SZ > maxIdx) {
6163+
/* @TODO extension data, in the process of storing attributes */
6164+
if (GetSkip(buf, maxIdx, &localIdx) != WS_SUCCESS) {
62036165
return WS_BUFFER_E;
62046166
}
6205-
ato32(buf + localIdx, &sz); localIdx += UINT32_SZ;
6206-
6207-
if (sz > 0) {
6208-
if (localIdx + sz > maxIdx) {
6209-
return WS_BUFFER_E;
6210-
}
6211-
/* @TODO extension data */
6212-
localIdx += sz;
6213-
}
62146167
}
62156168
}
62166169

@@ -6461,6 +6414,9 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh)
64616414

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

64666422
count--;
@@ -6477,8 +6433,9 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh)
64776433
n = tmp;
64786434

64796435
/* get filename size and name */
6480-
if (wolfSSH_SFTP_buffer_ato32(&state->buffer, &sz) !=
6481-
WS_SUCCESS) {
6436+
localIdx = wolfSSH_SFTP_buffer_idx(&state->buffer);
6437+
if (GetStringRef(&sz, &str, nameBuf, nameMax, &localIdx)
6438+
!= WS_SUCCESS) {
64826439
ret = WS_BUFFER_E;
64836440
break;
64846441
}
@@ -6490,24 +6447,13 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh)
64906447
ret = WS_MEMORY_E;
64916448
break;
64926449
}
6493-
6494-
if (wolfSSH_SFTP_buffer_idx(&state->buffer) + sz >
6495-
wolfSSH_SFTP_buffer_size(&state->buffer)) {
6496-
ret = WS_FATAL_ERROR;
6497-
break;
6498-
}
6499-
WMEMCPY(tmp->fName,
6500-
wolfSSH_SFTP_buffer_data(&state->buffer) +
6501-
wolfSSH_SFTP_buffer_idx(&state->buffer),
6502-
sz);
6503-
wolfSSH_SFTP_buffer_seek(&state->buffer,
6504-
wolfSSH_SFTP_buffer_idx(&state->buffer), sz);
6450+
WMEMCPY(tmp->fName, str, sz);
65056451
tmp->fName[sz] = '\0';
65066452
}
65076453

65086454
/* get longname size and name */
6509-
if (wolfSSH_SFTP_buffer_ato32(&state->buffer, &sz) !=
6510-
WS_SUCCESS) {
6455+
if (GetStringRef(&sz, &str, nameBuf, nameMax, &localIdx)
6456+
!= WS_SUCCESS) {
65116457
ret = WS_BUFFER_E;
65126458
break;
65136459
}
@@ -6519,27 +6465,13 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh)
65196465
ret = WS_MEMORY_E;
65206466
break;
65216467
}
6522-
6523-
if (wolfSSH_SFTP_buffer_idx(&state->buffer) + sz >
6524-
wolfSSH_SFTP_buffer_size(&state->buffer)) {
6525-
ret = WS_FATAL_ERROR;
6526-
break;
6527-
}
6528-
WMEMCPY(tmp->lName,
6529-
wolfSSH_SFTP_buffer_data(&state->buffer) +
6530-
wolfSSH_SFTP_buffer_idx(&state->buffer),
6531-
sz);
6532-
wolfSSH_SFTP_buffer_seek(&state->buffer,
6533-
wolfSSH_SFTP_buffer_idx(&state->buffer), sz);
6468+
WMEMCPY(tmp->lName, str, sz);
65346469
tmp->lName[sz] = '\0';
65356470
}
65366471

65376472
/* get attributes */
6538-
localIdx = wolfSSH_SFTP_buffer_idx(&state->buffer);
65396473
ret = SFTP_ParseAttributes_buffer(ssh, &tmp->atrb,
6540-
wolfSSH_SFTP_buffer_data(&state->buffer),
6541-
&localIdx,
6542-
wolfSSH_SFTP_buffer_size(&state->buffer));
6474+
nameBuf, &localIdx, nameMax);
65436475
wolfSSH_SFTP_buffer_seek(&state->buffer, 0, localIdx);
65446476
if (ret != WS_SUCCESS) {
65456477
break;

0 commit comments

Comments
 (0)