Skip to content

Commit 3e97c3c

Browse files
kfarnungkfarnung
committed
[MERGE #2329 @kfarnung] Adding bounds checking to the TTD string manipulation
Merge pull request #2329 from kfarnung:ttdstrings The string manipulation wasn't ensuring that the operation would not exceed the bounds of the buffer. Resolves #2305
2 parents 0a64a1c + 89a43bf commit 3e97c3c

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)