Skip to content

Commit 377aecd

Browse files
lucasssvazlucasssvaz
committed
fix(vfs): Fix TOCTOU vulnerability
1 parent 2d0b267 commit 377aecd

File tree

1 file changed

+73
-107
lines changed

1 file changed

+73
-107
lines changed

‎libraries/FS/src/vfs_api.cpp‎

Lines changed: 73 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
// limitations under the License.
1414

1515
#include "vfs_api.h"
16+
#include <errno.h>
17+
#include <stdlib.h>
18+
#include <string.h>
1619

1720
using namespace fs;
1821

@@ -38,67 +41,49 @@ FileImplPtr VFSImpl::open(const char *fpath, const char *mode, const bool create
3841
strcpy(temp, _mountpoint);
3942
strcat(temp, fpath);
4043

41-
struct stat st;
42-
//file found
43-
if (!stat(temp, &st)) {
44-
free(temp);
45-
if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode)) {
46-
return std::make_shared<VFSFileImpl>(this, fpath, mode);
47-
}
48-
log_e("%s has wrong mode 0x%08X", fpath, st.st_mode);
49-
return FileImplPtr();
50-
}
51-
52-
//try to open this as directory (might be mount point)
53-
DIR *d = opendir(temp);
54-
if (d) {
55-
closedir(d);
56-
free(temp);
57-
return std::make_shared<VFSFileImpl>(this, fpath, mode);
58-
}
59-
60-
//file not found but mode permits file creation without folder creation
61-
if ((mode && mode[0] != 'r') && (!create)) {
62-
free(temp);
63-
return std::make_shared<VFSFileImpl>(this, fpath, mode);
64-
}
65-
66-
////file not found but mode permits file creation and folder creation
67-
if ((mode && mode[0] != 'r') && create) {
68-
69-
char *token;
70-
char *folder = (char *)malloc(strlen(fpath) + 1);
71-
72-
int start_index = 0;
73-
int end_index = 0;
74-
75-
token = strchr(fpath + 1, '/');
76-
end_index = (token - fpath);
77-
78-
while (token != NULL) {
79-
memcpy(folder, fpath + start_index, end_index - start_index);
80-
folder[end_index - start_index] = '\0';
81-
82-
if (!VFSImpl::mkdir(folder)) {
83-
log_e("Creating folder: %s failed!", folder);
84-
return FileImplPtr();
44+
// Try to open as file first - let the file operation handle errors
45+
if (mode && mode[0] != 'r') {
46+
// For write modes, attempt to create directories if needed
47+
if (create) {
48+
char *token;
49+
char *folder = (char *)malloc(strlen(fpath) + 1);
50+
51+
int start_index = 0;
52+
int end_index = 0;
53+
54+
token = strchr(fpath + 1, '/');
55+
end_index = (token - fpath);
56+
57+
while (token != NULL) {
58+
memcpy(folder, fpath + start_index, end_index - start_index);
59+
folder[end_index - start_index] = '\0';
60+
61+
if (!VFSImpl::mkdir(folder)) {
62+
log_e("Creating folder: %s failed!", folder);
63+
free(folder);
64+
free(temp);
65+
return FileImplPtr();
66+
}
67+
68+
token = strchr(token + 1, '/');
69+
if (token != NULL) {
70+
end_index = (token - fpath);
71+
memset(folder, 0, strlen(folder));
72+
}
8573
}
8674

87-
token = strchr(token + 1, '/');
88-
if (token != NULL) {
89-
end_index = (token - fpath);
90-
memset(folder, 0, strlen(folder));
91-
}
75+
free(folder);
9276
}
9377

94-
free(folder);
78+
// Try to open the file directly - let fopen handle errors
9579
free(temp);
9680
return std::make_shared<VFSFileImpl>(this, fpath, mode);
9781
}
9882

99-
log_e("%s does not exist, no permits for creation", temp);
83+
// For read mode, let the VFSFileImpl constructor handle the file opening
84+
// This avoids the TOCTOU race condition while maintaining proper functionality
10085
free(temp);
101-
return FileImplPtr();
86+
return std::make_shared<VFSFileImpl>(this, fpath, mode);
10287
}
10388

10489
bool VFSImpl::exists(const char *fpath) {
@@ -125,10 +110,7 @@ bool VFSImpl::rename(const char *pathFrom, const char *pathTo) {
125110
log_e("bad arguments");
126111
return false;
127112
}
128-
if (!exists(pathFrom)) {
129-
log_e("%s does not exists", pathFrom);
130-
return false;
131-
}
113+
132114
size_t mountpointLen = strlen(_mountpoint);
133115
char *temp1 = (char *)malloc(strlen(pathFrom) + mountpointLen + 1);
134116
if (!temp1) {
@@ -148,6 +130,7 @@ bool VFSImpl::rename(const char *pathFrom, const char *pathTo) {
148130
strcpy(temp2, _mountpoint);
149131
strcat(temp2, pathTo);
150132

133+
// Let rename() handle the error if source doesn't exist
151134
auto rc = ::rename(temp1, temp2);
152135
free(temp1);
153136
free(temp2);
@@ -165,16 +148,6 @@ bool VFSImpl::remove(const char *fpath) {
165148
return false;
166149
}
167150

168-
VFSFileImpl f(this, fpath, "r");
169-
if (!f || f.isDirectory()) {
170-
if (f) {
171-
f.close();
172-
}
173-
log_e("%s does not exists or is directory", fpath);
174-
return false;
175-
}
176-
f.close();
177-
178151
char *temp = (char *)malloc(strlen(fpath) + strlen(_mountpoint) + 1);
179152
if (!temp) {
180153
log_e("malloc failed");
@@ -184,6 +157,7 @@ bool VFSImpl::remove(const char *fpath) {
184157
strcpy(temp, _mountpoint);
185158
strcat(temp, fpath);
186159

160+
// Let unlink() handle the error if file doesn't exist
187161
auto rc = unlink(temp);
188162
free(temp);
189163
return rc == 0;
@@ -231,16 +205,6 @@ bool VFSImpl::rmdir(const char *fpath) {
231205
return false;
232206
}
233207

234-
VFSFileImpl f(this, fpath, "r");
235-
if (!f || !f.isDirectory()) {
236-
if (f) {
237-
f.close();
238-
}
239-
log_e("%s does not exists or is a file", fpath);
240-
return false;
241-
}
242-
f.close();
243-
244208
char *temp = (char *)malloc(strlen(fpath) + strlen(_mountpoint) + 1);
245209
if (!temp) {
246210
log_e("malloc failed");
@@ -250,6 +214,7 @@ bool VFSImpl::rmdir(const char *fpath) {
250214
strcpy(temp, _mountpoint);
251215
strcat(temp, fpath);
252216

217+
// Let rmdir() handle the error if directory doesn't exist
253218
auto rc = ::rmdir(temp);
254219
free(temp);
255220
return rc == 0;
@@ -271,29 +236,30 @@ VFSFileImpl::VFSFileImpl(VFSImpl *fs, const char *fpath, const char *mode) : _fs
271236
return;
272237
}
273238

274-
if (!stat(temp, &_stat)) {
275-
//file found
276-
if (S_ISREG(_stat.st_mode)) {
277-
_isDirectory = false;
278-
_f = fopen(temp, mode);
279-
if (!_f) {
280-
log_e("fopen(%s) failed", temp);
281-
}
282-
if (_f && (_stat.st_blksize == 0)) {
283-
setvbuf(_f, NULL, _IOFBF, DEFAULT_FILE_BUFFER_SIZE);
284-
}
285-
} else if (S_ISDIR(_stat.st_mode)) {
286-
_isDirectory = true;
287-
_d = opendir(temp);
288-
if (!_d) {
289-
log_e("opendir(%s) failed", temp);
239+
// For read mode, check if file exists first to determine type
240+
if (!mode || mode[0] == 'r') {
241+
if (!stat(temp, &_stat)) {
242+
//file found
243+
if (S_ISREG(_stat.st_mode)) {
244+
_isDirectory = false;
245+
_f = fopen(temp, mode);
246+
if (!_f) {
247+
log_e("fopen(%s) failed", temp);
248+
}
249+
if (_f && (_stat.st_blksize == 0)) {
250+
setvbuf(_f, NULL, _IOFBF, DEFAULT_FILE_BUFFER_SIZE);
251+
}
252+
} else if (S_ISDIR(_stat.st_mode)) {
253+
_isDirectory = true;
254+
_d = opendir(temp);
255+
if (!_d) {
256+
log_e("opendir(%s) failed", temp);
257+
}
258+
} else {
259+
log_e("Unknown type 0x%08X for file %s", ((_stat.st_mode) & _IFMT), temp);
290260
}
291261
} else {
292-
log_e("Unknown type 0x%08X for file %s", ((_stat.st_mode) & _IFMT), temp);
293-
}
294-
} else {
295-
//file not found
296-
if (!mode || mode[0] == 'r') {
262+
//file not found
297263
//try to open as directory
298264
_d = opendir(temp);
299265
if (_d) {
@@ -302,16 +268,16 @@ VFSFileImpl::VFSFileImpl(VFSImpl *fs, const char *fpath, const char *mode) : _fs
302268
_isDirectory = false;
303269
//log_w("stat(%s) failed", temp);
304270
}
305-
} else {
306-
//lets create this new file
307-
_isDirectory = false;
308-
_f = fopen(temp, mode);
309-
if (!_f) {
310-
log_e("fopen(%s) failed", temp);
311-
}
312-
if (_f && (_stat.st_blksize == 0)) {
313-
setvbuf(_f, NULL, _IOFBF, DEFAULT_FILE_BUFFER_SIZE);
314-
}
271+
}
272+
} else {
273+
//lets create this new file
274+
_isDirectory = false;
275+
_f = fopen(temp, mode);
276+
if (!_f) {
277+
log_e("fopen(%s) failed", temp);
278+
}
279+
if (_f && (_stat.st_blksize == 0)) {
280+
setvbuf(_f, NULL, _IOFBF, DEFAULT_FILE_BUFFER_SIZE);
315281
}
316282
}
317283
free(temp);

0 commit comments

Comments
 (0)