Skip to content

Commit 89a43bf

Browse files
kfarnungkfarnung
committed
Adding bounds checking to the TTD string manipulation
GetTTDDirectory() and the dependent functions weren't verifying the bounds of the buffer before writing. This change adds fail fast abort to the string manipulation since it's just a part of the ch test app.
1 parent 0a64a1c commit 89a43bf

File tree

1 file changed

+70
-39
lines changed

1 file changed

+70
-39
lines changed

‎bin/ch/Helpers.cpp‎

Lines changed: 70 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -35,36 +35,51 @@ void TTDHostInitFromUriBytes(TTDHostCharType* dst, const byte* uriBytes, size_t
3535
AssertMsg(wcslen(dst) == (uriBytesLength / sizeof(TTDHostCharType)), "We have an null in the uri or our math is wrong somewhere.");
3636
}
3737

38-
void TTDHostAppend(TTDHostCharType* dst, const TTDHostCharType* src)
38+
void TTDHostAppend(TTDHostCharType* dst, size_t dstLength, const TTDHostCharType* src)
3939
{
40-
size_t dpos = TTDHostStringLength(dst);
4140
size_t srcLength = TTDHostStringLength(src);
41+
size_t dpos = TTDHostStringLength(dst);
42+
Helpers::TTReportLastIOErrorAsNeeded(dpos < dstLength, "The end of the string already exceeds the buffer");
43+
4244
size_t srcByteLength = srcLength * sizeof(TTDHostCharType);
45+
size_t dstRemainingByteLength = (dstLength - dpos - 1) * sizeof(TTDHostCharType);
46+
Helpers::TTReportLastIOErrorAsNeeded(srcByteLength <= dstRemainingByteLength, "The source string must be able to fit within the destination buffer");
4347

44-
memcpy_s(dst + dpos, srcByteLength, src, srcByteLength);
48+
memcpy_s(dst + dpos, dstRemainingByteLength, src, srcByteLength);
4549
dst[dpos + srcLength] = _u('\0');
4650
}
4751

48-
void TTDHostAppendWChar(TTDHostCharType* dst, const wchar* src)
52+
void TTDHostAppendWChar(TTDHostCharType* dst, size_t dstLength, const wchar* src)
4953
{
50-
size_t dpos = TTDHostStringLength(dst);
5154
size_t srcLength = wcslen(src);
55+
size_t dpos = TTDHostStringLength(dst);
56+
Helpers::TTReportLastIOErrorAsNeeded(dpos < dstLength, "The end of the string already exceeds the buffer");
57+
58+
size_t dstRemainingLength = dstLength - dpos - 1;
59+
Helpers::TTReportLastIOErrorAsNeeded(srcLength <= dstRemainingLength, "The source string must be able to fit within the destination buffer");
5260

5361
for(size_t i = 0; i < srcLength; ++i)
5462
{
5563
dst[dpos + i] = (char16)src[i];
5664
}
65+
5766
dst[dpos + srcLength] = _u('\0');
5867
}
5968

60-
void TTDHostAppendAscii(TTDHostCharType* dst, const char* src)
69+
void TTDHostAppendAscii(TTDHostCharType* dst, size_t dstLength, const char* src)
6170
{
62-
size_t dpos = TTDHostStringLength(dst);
6371
size_t srcLength = strlen(src);
72+
size_t dpos = TTDHostStringLength(dst);
73+
Helpers::TTReportLastIOErrorAsNeeded(dpos < dstLength, "The end of the string already exceeds the buffer");
74+
75+
size_t dstRemainingLength = dstLength - dpos - 1;
76+
Helpers::TTReportLastIOErrorAsNeeded(srcLength <= dstRemainingLength, "The source string must be able to fit within the destination buffer");
77+
6478
for(size_t i = 0; i < srcLength; ++i)
6579
{
6680
dst[dpos + i] = (char16)src[i];
6781
}
82+
6883
dst[dpos + srcLength] = _u('\0');
6984
}
7085

@@ -80,7 +95,7 @@ void TTDHostBuildCurrentExeDirectory(TTDHostCharType* path, size_t pathBufferLen
8095
}
8196
exePath[i + 1] = _u('\0');
8297

83-
TTDHostAppendWChar(path, exePath);
98+
TTDHostAppendWChar(path, pathBufferLength, exePath);
8499
}
85100

86101
JsTTDStreamHandle TTDHostOpen(const TTDHostCharType* path, bool isWrite)
@@ -91,8 +106,8 @@ JsTTDStreamHandle TTDHostOpen(const TTDHostCharType* path, bool isWrite)
91106
return (JsTTDStreamHandle)res;
92107
}
93108

94-
#define TTDHostCWD(dst) _wgetcwd(dst, MAX_PATH)
95-
#define TTDDoPathInit(dst)
109+
#define TTDHostCWD(dst, dstLength) _wgetcwd(dst, dstLength)
110+
#define TTDDoPathInit(dst, dstLength)
96111
#define TTDHostTok(opath, TTDHostPathSeparator, context) wcstok_s(opath, TTDHostPathSeparator, context)
97112
#define TTDHostStat(cpath, statVal) _wstat(cpath, statVal)
98113

@@ -148,30 +163,44 @@ void TTDHostInitFromUriBytes(TTDHostCharType* dst, const byte* uriBytes, size_t
148163
AssertMsg(TTDHostStringLength(dst) == (uriBytesLength / sizeof(TTDHostCharType)), "We have an null in the uri or our math is wrong somewhere.");
149164
}
150165

151-
void TTDHostAppend(TTDHostCharType* dst, const TTDHostCharType* src)
166+
void TTDHostAppend(TTDHostCharType* dst, size_t dstLength, const TTDHostCharType* src)
152167
{
153-
size_t dpos = TTDHostStringLength(dst);
154168
size_t srcLength = TTDHostStringLength(src);
169+
size_t dpos = TTDHostStringLength(dst);
170+
Helpers::TTReportLastIOErrorAsNeeded(dpos < dstLength, "The end of the string already exceeds the buffer");
171+
155172
size_t srcByteLength = srcLength * sizeof(TTDHostCharType);
173+
size_t dstRemainingByteLength = (dstLength - dpos - 1) * sizeof(TTDHostCharType);
174+
Helpers::TTReportLastIOErrorAsNeeded(srcByteLength <= dstRemainingByteLength, "The source string must be able to fit within the destination buffer");
156175

157-
memcpy_s(dst + dpos, srcByteLength, src, srcByteLength);
176+
memcpy_s(dst + dpos, dstRemainingByteLength, src, srcByteLength);
158177
dst[dpos + srcLength] = '\0';
159178
}
160179

161-
void TTDHostAppendWChar(TTDHostCharType* dst, const wchar* src)
180+
void TTDHostAppendWChar(TTDHostCharType* dst, size_t dstLength, const wchar* src)
162181
{
163-
size_t dpos = TTDHostStringLength(dst);
164182
size_t srcLength = wcslen(src);
183+
size_t dpos = TTDHostStringLength(dst);
184+
Helpers::TTReportLastIOErrorAsNeeded(dpos < dstLength, "The end of the string already exceeds the buffer");
185+
186+
size_t dstRemainingLength = dstLength - dpos - 1;
187+
Helpers::TTReportLastIOErrorAsNeeded(srcLength <= dstRemainingLength, "The source string must be able to fit within the destination buffer");
188+
189+
// TODO - analyze this function further
165190
utf8::EncodeIntoAndNullTerminate(dst + dpos, src, srcLength);
166191
}
167192

168-
void TTDHostAppendAscii(TTDHostCharType* dst, const char* src)
193+
void TTDHostAppendAscii(TTDHostCharType* dst, size_t dstLength, const char* src)
169194
{
170-
size_t dpos = TTDHostStringLength(dst);
171195
size_t srcLength = strlen(src);
196+
size_t dpos = TTDHostStringLength(dst);
197+
Helpers::TTReportLastIOErrorAsNeeded(dpos < dstLength, "The end of the string already exceeds the buffer");
198+
172199
size_t srcByteLength = srcLength * sizeof(TTDHostCharType);
200+
size_t dstRemainingByteLength = (dstLength - dpos - 1) * sizeof(TTDHostCharType);
201+
Helpers::TTReportLastIOErrorAsNeeded(srcByteLength <= dstRemainingByteLength, "The source string must be able to fit within the destination buffer");
173202

174-
memcpy_s(dst + dpos, srcByteLength, src, srcByteLength);
203+
memcpy_s(dst + dpos, dstRemainingByteLength, src, srcByteLength);
175204
dst[dpos + srcLength] = '\0';
176205
}
177206

@@ -192,18 +221,19 @@ void TTDHostBuildCurrentExeDirectory(TTDHostCharType* path, size_t pathBufferLen
192221
{
193222
--i;
194223
}
224+
195225
exePath[i + 1] = '\0';
196226

197-
TTDHostAppend(path, exePath);
227+
TTDHostAppend(path, pathBufferLength, exePath);
198228
}
199229

200230
JsTTDStreamHandle TTDHostOpen(const TTDHostCharType* path, bool isWrite)
201231
{
202232
return (JsTTDStreamHandle)fopen(TTDHostCharConvert(path), isWrite ? "w+b" : "r+b");
203233
}
204234

205-
#define TTDHostCWD(dst) TTDHostUtf8CharConvert(getcwd(TTDHostCharConvert(dst), MAX_PATH))
206-
#define TTDDoPathInit(dst) TTDHostAppend(dst, TTDHostPathSeparator)
235+
#define TTDHostCWD(dst, dstLength) TTDHostUtf8CharConvert(getcwd(TTDHostCharConvert(dst), dstLength))
236+
#define TTDDoPathInit(dst, dstLength) TTDHostAppend(dst, dstLength, TTDHostPathSeparator)
207237
#define TTDHostTok(opath, TTDHostPathSeparator, context) TTDHostUtf8CharConvert(strtok(TTDHostCharConvert(opath), TTDHostCharConvert(TTDHostPathSeparator)))
208238
#define TTDHostStat(cpath, statVal) stat(TTDHostCharConvert(cpath), statVal)
209239

@@ -504,6 +534,7 @@ HRESULT Helpers::LoadBinaryFile(LPCSTR filename, LPCSTR& contents, UINT& lengthB
504534

505535
return hr;
506536
}
537+
507538
void Helpers::TTReportLastIOErrorAsNeeded(BOOL ok, const char* msg)
508539
{
509540
if(!ok)
@@ -530,19 +561,19 @@ void Helpers::CreateDirectoryIfNeeded(size_t uriByteLength, const byte* uriBytes
530561

531562
TTDHostCharType cpath[MAX_PATH];
532563
TTDHostInitEmpty(cpath);
533-
TTDDoPathInit(cpath);
564+
TTDDoPathInit(cpath, MAX_PATH);
534565

535566
TTDHostStatType statVal;
536567
TTDHostCharType* context = nullptr;
537568
TTDHostCharType* token = TTDHostTok(opath, TTDHostPathSeparator, &context);
538-
TTDHostAppend(cpath, token);
569+
TTDHostAppend(cpath, MAX_PATH, token);
539570

540571
//At least 1 part of the path must exist so iterate until we find it
541572
while(TTDHostStat(cpath, &statVal) == -1)
542573
{
543574
token = TTDHostTok(nullptr, TTDHostPathSeparator, &context);
544-
TTDHostAppend(cpath, TTDHostPathSeparator);
545-
TTDHostAppend(cpath, token);
575+
TTDHostAppend(cpath, MAX_PATH, TTDHostPathSeparator);
576+
TTDHostAppend(cpath, MAX_PATH, token);
546577
}
547578

548579
//Now continue until we hit the part that doesn't exist (or the end of the path)
@@ -551,8 +582,8 @@ void Helpers::CreateDirectoryIfNeeded(size_t uriByteLength, const byte* uriBytes
551582
token = TTDHostTok(nullptr, TTDHostPathSeparator, &context);
552583
if(token != nullptr)
553584
{
554-
TTDHostAppend(cpath, TTDHostPathSeparator);
555-
TTDHostAppend(cpath, token);
585+
TTDHostAppend(cpath, MAX_PATH, TTDHostPathSeparator);
586+
TTDHostAppend(cpath, MAX_PATH, token);
556587
}
557588
}
558589

@@ -569,8 +600,8 @@ void Helpers::CreateDirectoryIfNeeded(size_t uriByteLength, const byte* uriBytes
569600
token = TTDHostTok(nullptr, TTDHostPathSeparator, &context);
570601
if(token != nullptr)
571602
{
572-
TTDHostAppend(cpath, TTDHostPathSeparator);
573-
TTDHostAppend(cpath, token);
603+
TTDHostAppend(cpath, MAX_PATH, TTDHostPathSeparator);
604+
TTDHostAppend(cpath, MAX_PATH, token);
574605
}
575606
}
576607
}
@@ -582,7 +613,7 @@ void Helpers::CleanDirectory(size_t uriByteLength, const byte* uriBytes)
582613

583614
TTDHostCharType strPattern[MAX_PATH];
584615
TTDHostInitFromUriBytes(strPattern, uriBytes, uriByteLength);
585-
TTDHostAppendAscii(strPattern, "*.*");
616+
TTDHostAppendAscii(strPattern, MAX_PATH, "*.*");
586617

587618
hFile = TTDHostFindFirst(strPattern, &FileInformation);
588619
if(hFile != TTDHostFindInvalid)
@@ -593,7 +624,7 @@ void Helpers::CleanDirectory(size_t uriByteLength, const byte* uriBytes)
593624
{
594625
TTDHostCharType strFilePath[MAX_PATH];
595626
TTDHostInitFromUriBytes(strFilePath, uriBytes, uriByteLength);
596-
TTDHostAppend(strFilePath, TTDHostDirInfoName(FileInformation));
627+
TTDHostAppend(strFilePath, MAX_PATH, TTDHostDirInfoName(FileInformation));
597628

598629
// Set file attributes
599630
int statusch = TTDHostCHMod(strFilePath, S_IREAD | S_IWRITE);
@@ -616,27 +647,27 @@ void Helpers::GetTTDDirectory(const wchar* curi, size_t* uriByteLength, byte* ur
616647

617648
if(curi[0] != _u('~'))
618649
{
619-
TTDHostCharType* status = TTDHostCWD(turi);
650+
TTDHostCharType* status = TTDHostCWD(turi, MAX_PATH);
620651
Helpers::TTReportLastIOErrorAsNeeded(status != nullptr, "Failed to chmod directory");
621652

622-
TTDHostAppend(turi, TTDHostPathSeparator);
653+
TTDHostAppend(turi, MAX_PATH, TTDHostPathSeparator);
623654

624-
TTDHostAppendWChar(turi, curi);
655+
TTDHostAppendWChar(turi, MAX_PATH, curi);
625656
}
626657
else
627658
{
628659
TTDHostBuildCurrentExeDirectory(turi, MAX_PATH);
629660

630-
TTDHostAppendAscii(turi, "_ttdlog");
631-
TTDHostAppend(turi, TTDHostPathSeparator);
661+
TTDHostAppendAscii(turi, MAX_PATH, "_ttdlog");
662+
TTDHostAppend(turi, MAX_PATH, TTDHostPathSeparator);
632663

633-
TTDHostAppendWChar(turi, curi + 1);
664+
TTDHostAppendWChar(turi, MAX_PATH, curi + 1);
634665
}
635666

636667
//add a path separator if one is not already present
637668
if(curi[wcslen(curi) - 1] != (wchar)TTDHostPathSeparatorChar)
638669
{
639-
TTDHostAppend(turi, TTDHostPathSeparator);
670+
TTDHostAppend(turi, MAX_PATH, TTDHostPathSeparator);
640671
}
641672

642673
size_t turiLength = TTDHostStringLength(turi);
@@ -665,7 +696,7 @@ JsTTDStreamHandle CALLBACK Helpers::TTCreateStreamCallback(size_t uriByteLength,
665696
void* res = nullptr;
666697
TTDHostCharType path[MAX_PATH];
667698
TTDHostInitFromUriBytes(path, uriBytes, uriByteLength);
668-
TTDHostAppendAscii(path, asciiResourceName);
699+
TTDHostAppendAscii(path, MAX_PATH, asciiResourceName);
669700

670701
res = TTDHostOpen(path, write);
671702
if(res == nullptr)

0 commit comments

Comments
 (0)