Skip to content

Commit 2a56654

Browse files
raysan5raysan5
committed
ADDED: Multiply security checks to avoid crashes on wrongly provided string data #4751
- REVIEWED: Checking `NULL` input on functions getting `const char *text`, to avoid crashes - REVIEWED: `strcpy()` usage, prioritize `strncpy()` with limited copy to buffer size - REPLACED: `strlen()` by `TextLength()` on [rtext] module - REVIEWED: Replaced some early returns (but keeping others, for easier code following)
1 parent 71a35f6 commit 2a56654

File tree

8 files changed

+275
-234
lines changed

8 files changed

+275
-234
lines changed

‎src/raudio.c‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,7 +1140,7 @@ bool ExportWaveAsCode(Wave wave, const char *fileName)
11401140

11411141
// Get file name from path and convert variable name to uppercase
11421142
char varFileName[256] = { 0 };
1143-
strcpy(varFileName, GetFileNameWithoutExt(fileName));
1143+
strncpy(varFileName, GetFileNameWithoutExt(fileName), 256 - 1);
11441144
for (int i = 0; varFileName[i] != '\0'; i++) if (varFileName[i] >= 'a' && varFileName[i] <= 'z') { varFileName[i] = varFileName[i] - 32; }
11451145

11461146
// Add wave information
@@ -2739,11 +2739,13 @@ static const char *GetFileExtension(const char *fileName)
27392739
return dot;
27402740
}
27412741

2742-
// String pointer reverse break: returns right-most occurrence of charset in s
2743-
static const char *strprbrk(const char *s, const char *charset)
2742+
// String pointer reverse break: returns right-most occurrence of charset in text
2743+
static const char *strprbrk(const char *text, const char *charset)
27442744
{
27452745
const char *latestMatch = NULL;
2746-
for (; s = strpbrk(s, charset), s != NULL; latestMatch = s++) { }
2746+
2747+
for (; (text != NULL) && (text = strpbrk(text, charset)); latestMatch = text++) { }
2748+
27472749
return latestMatch;
27482750
}
27492751

@@ -2766,7 +2768,7 @@ static const char *GetFileNameWithoutExt(const char *filePath)
27662768
static char fileName[MAX_FILENAMEWITHOUTEXT_LENGTH] = { 0 };
27672769
memset(fileName, 0, MAX_FILENAMEWITHOUTEXT_LENGTH);
27682770

2769-
if (filePath != NULL) strcpy(fileName, GetFileName(filePath)); // Get filename with extension
2771+
if (filePath != NULL) strncpy(fileName, GetFileName(filePath), MAX_FILENAMEWITHOUTEXT_LENGTH - 1); // Get filename with extension
27702772

27712773
int size = (int)strlen(fileName); // Get size in bytes
27722774

@@ -2864,7 +2866,8 @@ static bool SaveFileText(const char *fileName, char *text)
28642866

28652867
if (file != NULL)
28662868
{
2867-
int count = fprintf(file, "%s", text);
2869+
int count = 0;
2870+
if (text != NULL) count = fprintf(file, "%s", text);
28682871

28692872
if (count == 0) TRACELOG(LOG_WARNING, "FILEIO: [%s] Failed to write text file", fileName);
28702873
else TRACELOG(LOG_INFO, "FILEIO: [%s] Text file saved successfully", fileName);

‎src/raylib.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1145,7 +1145,7 @@ RLAPI const char *GetPrevDirectoryPath(const char *dirPath); // Get previ
11451145
RLAPI const char *GetWorkingDirectory(void); // Get current working directory (uses static string)
11461146
RLAPI const char *GetApplicationDirectory(void); // Get the directory of the running application (uses static string)
11471147
RLAPI int MakeDirectory(const char *dirPath); // Create directories (including full path requested), returns 0 on success
1148-
RLAPI bool ChangeDirectory(const char *dir); // Change working directory, return true on success
1148+
RLAPI bool ChangeDirectory(const char *dirPath); // Change working directory, return true on success
11491149
RLAPI bool IsPathFile(const char *path); // Check if a given path is a file or a directory
11501150
RLAPI bool IsFileNameValid(const char *fileName); // Check if fileName is valid for the platform/OS
11511151
RLAPI FilePathList LoadDirectoryFiles(const char *dirPath); // Load directory filepaths

‎src/rcore.c‎

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@
113113

114114
#include <stdlib.h> // Required for: srand(), rand(), atexit()
115115
#include <stdio.h> // Required for: sprintf() [Used in OpenURL()]
116-
#include <string.h> // Required for: strlen(), strcpy(), strcmp(), strrchr(), memset()
116+
#include <string.h> // Required for: strlen(), strncpy(), strcmp(), strrchr(), memset()
117117
#include <time.h> // Required for: time() [Used in InitTimer()]
118118
#include <math.h> // Required for: tan() [Used in BeginMode3D()], atan2f() [Used in LoadVrStereoConfig()]
119119

@@ -1837,8 +1837,8 @@ void TakeScreenshot(const char *fileName)
18371837
unsigned char *imgData = rlReadScreenPixels((int)((float)CORE.Window.render.width*scale.x), (int)((float)CORE.Window.render.height*scale.y));
18381838
Image image = { imgData, (int)((float)CORE.Window.render.width*scale.x), (int)((float)CORE.Window.render.height*scale.y), 1, PIXELFORMAT_UNCOMPRESSED_R8G8B8A8 };
18391839

1840-
char path[512] = { 0 };
1841-
strcpy(path, TextFormat("%s/%s", CORE.Storage.basePath, fileName));
1840+
char path[MAX_FILEPATH_LENGTH] = { 0 };
1841+
strncpy(path, TextFormat("%s/%s", CORE.Storage.basePath, fileName), MAX_FILEPATH_LENGTH - 1);
18421842

18431843
ExportImage(image, path); // WARNING: Module required: rtextures
18441844
RL_FREE(imgData);
@@ -2022,7 +2022,7 @@ bool IsFileExtension(const char *fileName, const char *ext)
20222022
int extLen = (int)strlen(ext);
20232023
char *extList = (char *)RL_CALLOC(extLen + 1, 1);
20242024
char *extListPtrs[MAX_FILE_EXTENSIONS] = { 0 };
2025-
strcpy(extList, ext);
2025+
strncpy(extList, ext, extLen);
20262026
extListPtrs[0] = extList;
20272027

20282028
for (int i = 0; i < extLen; i++)
@@ -2130,11 +2130,11 @@ const char *GetFileExtension(const char *fileName)
21302130
}
21312131

21322132
// String pointer reverse break: returns right-most occurrence of charset in s
2133-
static const char *strprbrk(const char *s, const char *charset)
2133+
static const char *strprbrk(const char *text, const char *charset)
21342134
{
21352135
const char *latestMatch = NULL;
21362136

2137-
for (; s = strpbrk(s, charset), s != NULL; latestMatch = s++) { }
2137+
for (; (text != NULL) && (text = strpbrk(text, charset)); latestMatch = text++) { }
21382138

21392139
return latestMatch;
21402140
}
@@ -2161,7 +2161,7 @@ const char *GetFileNameWithoutExt(const char *filePath)
21612161

21622162
if (filePath != NULL)
21632163
{
2164-
strcpy(fileName, GetFileName(filePath)); // Get filename.ext without path
2164+
strncpy(fileName, GetFileName(filePath), MAX_FILENAME_LENGTH - 1); // Get filename.ext without path
21652165
int size = (int)strlen(fileName); // Get size in bytes
21662166

21672167
for (int i = size; i > 0; i--) // Reverse search '.'
@@ -2233,7 +2233,7 @@ const char *GetPrevDirectoryPath(const char *dirPath)
22332233
memset(prevDirPath, 0, MAX_FILEPATH_LENGTH);
22342234
int pathLen = (int)strlen(dirPath);
22352235

2236-
if (pathLen <= 3) strcpy(prevDirPath, dirPath);
2236+
if (pathLen <= 3) strncpy(prevDirPath, dirPath, MAX_FILEPATH_LENGTH - 1);
22372237

22382238
for (int i = (pathLen - 1); (i >= 0) && (pathLen > 3); i--)
22392239
{
@@ -2472,12 +2472,12 @@ int MakeDirectory(const char *dirPath)
24722472
}
24732473

24742474
// Change working directory, returns true on success
2475-
bool ChangeDirectory(const char *dir)
2475+
bool ChangeDirectory(const char *dirPath)
24762476
{
2477-
bool result = CHDIR(dir);
2477+
bool result = CHDIR(dirPath);
24782478

2479-
if (result != 0) TRACELOG(LOG_WARNING, "SYSTEM: Failed to change to directory: %s", dir);
2480-
else TRACELOG(LOG_INFO, "SYSTEM: Working Directory: %s", dir);
2479+
if (result != 0) TRACELOG(LOG_WARNING, "SYSTEM: Failed to change to directory: %s", dirPath);
2480+
else TRACELOG(LOG_INFO, "SYSTEM: Working Directory: %s", dirPath);
24812481

24822482
return (result == 0);
24832483
}
@@ -2708,6 +2708,9 @@ unsigned char *DecodeDataBase64(const char *text, int *outputSize)
27082708
['0'] = 52, ['1'] = 53, ['2'] = 54, ['3'] = 55, ['4'] = 56, ['5'] = 57, ['6'] = 58, ['7'] = 59,
27092709
['8'] = 60, ['9'] = 61, ['+'] = 62, ['/'] = 63
27102710
};
2711+
2712+
*outputSize = 0;
2713+
if (text == NULL) return NULL;
27112714

27122715
// Compute expected size and padding
27132716
int dataSize = (int)strlen(text); // WARNING: Expecting NULL terminated strings!
@@ -3952,22 +3955,22 @@ static void ScanDirectoryFiles(const char *basePath, FilePathList *files, const
39523955
{
39533956
if (IsFileExtension(path, filter))
39543957
{
3955-
strcpy(files->paths[files->count], path);
3958+
strncpy(files->paths[files->count], path, MAX_FILEPATH_LENGTH - 1);
39563959
files->count++;
39573960
}
39583961
}
39593962
else
39603963
{
39613964
if (strstr(filter, DIRECTORY_FILTER_TAG) != NULL)
39623965
{
3963-
strcpy(files->paths[files->count], path);
3966+
strncpy(files->paths[files->count], path, MAX_FILEPATH_LENGTH - 1);
39643967
files->count++;
39653968
}
39663969
}
39673970
}
39683971
else
39693972
{
3970-
strcpy(files->paths[files->count], path);
3973+
strncpy(files->paths[files->count], path, MAX_FILEPATH_LENGTH - 1);
39713974
files->count++;
39723975
}
39733976
}
@@ -4011,13 +4014,13 @@ static void ScanDirectoryFilesRecursively(const char *basePath, FilePathList *fi
40114014
{
40124015
if (IsFileExtension(path, filter))
40134016
{
4014-
strcpy(files->paths[files->count], path);
4017+
strncpy(files->paths[files->count], path, MAX_FILEPATH_LENGTH - 1);
40154018
files->count++;
40164019
}
40174020
}
40184021
else
40194022
{
4020-
strcpy(files->paths[files->count], path);
4023+
strncpy(files->paths[files->count], path, MAX_FILEPATH_LENGTH - 1);
40214024
files->count++;
40224025
}
40234026

@@ -4031,7 +4034,7 @@ static void ScanDirectoryFilesRecursively(const char *basePath, FilePathList *fi
40314034
{
40324035
if ((filter != NULL) && (strstr(filter, DIRECTORY_FILTER_TAG) != NULL))
40334036
{
4034-
strcpy(files->paths[files->count], path);
4037+
strncpy(files->paths[files->count], path, MAX_FILEPATH_LENGTH - 1);
40354038
files->count++;
40364039
}
40374040

@@ -4334,23 +4337,26 @@ const char *TextFormat(const char *text, ...)
43344337

43354338
char *currentBuffer = buffers[index];
43364339
memset(currentBuffer, 0, MAX_TEXT_BUFFER_LENGTH); // Clear buffer before using
4340+
4341+
if (text != NULL)
4342+
{
4343+
va_list args;
4344+
va_start(args, text);
4345+
int requiredByteCount = vsnprintf(currentBuffer, MAX_TEXT_BUFFER_LENGTH, text, args);
4346+
va_end(args);
43374347

4338-
va_list args;
4339-
va_start(args, text);
4340-
int requiredByteCount = vsnprintf(currentBuffer, MAX_TEXT_BUFFER_LENGTH, text, args);
4341-
va_end(args);
4348+
// If requiredByteCount is larger than the MAX_TEXT_BUFFER_LENGTH, then overflow occurred
4349+
if (requiredByteCount >= MAX_TEXT_BUFFER_LENGTH)
4350+
{
4351+
// Inserting "..." at the end of the string to mark as truncated
4352+
char *truncBuffer = buffers[index] + MAX_TEXT_BUFFER_LENGTH - 4; // Adding 4 bytes = "...\0"
4353+
snprintf(truncBuffer, 4, "...");
4354+
}
43424355

4343-
// If requiredByteCount is larger than the MAX_TEXT_BUFFER_LENGTH, then overflow occurred
4344-
if (requiredByteCount >= MAX_TEXT_BUFFER_LENGTH)
4345-
{
4346-
// Inserting "..." at the end of the string to mark as truncated
4347-
char *truncBuffer = buffers[index] + MAX_TEXT_BUFFER_LENGTH - 4; // Adding 4 bytes = "...\0"
4348-
snprintf(truncBuffer, 4, "...");
4356+
index += 1; // Move to next buffer for next function call
4357+
if (index >= MAX_TEXTFORMAT_BUFFERS) index = 0;
43494358
}
43504359

4351-
index += 1; // Move to next buffer for next function call
4352-
if (index >= MAX_TEXTFORMAT_BUFFERS) index = 0;
4353-
43544360
return currentBuffer;
43554361
}
43564362

‎src/rlgl.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2492,12 +2492,12 @@ void rlLoadExtensions(void *loader)
24922492
const char *extensions = (const char *)glGetString(GL_EXTENSIONS); // One big const string
24932493

24942494
// NOTE: We have to duplicate string because glGetString() returns a const string
2495-
int size = strlen(extensions) + 1; // Get extensions string size in bytes
2496-
char *extensionsDup = (char *)RL_CALLOC(size, sizeof(char));
2497-
strcpy(extensionsDup, extensions);
2495+
int extSize = (int)strlen(extensions); // Get extensions string size in bytes
2496+
char *extensionsDup = (char *)RL_CALLOC(extSize + 1, sizeof(char)); // Allocate space for copy with additional EOL byte
2497+
strncpy(extensionsDup, extensions, extSize);
24982498
extList[numExt] = extensionsDup;
24992499

2500-
for (int i = 0; i < size; i++)
2500+
for (int i = 0; i < extSize; i++)
25012501
{
25022502
if (extensionsDup[i] == ' ')
25032503
{

‎src/rmodels.c‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2060,7 +2060,7 @@ bool ExportMeshAsCode(Mesh mesh, const char *fileName)
20602060

20612061
// Get file name from path and convert variable name to uppercase
20622062
char varFileName[256] = { 0 };
2063-
strcpy(varFileName, GetFileNameWithoutExt(fileName));
2063+
strncpy(varFileName, GetFileNameWithoutExt(fileName), 256 - 1); // NOTE: Using function provided by [rcore] module
20642064
for (int i = 0; varFileName[i] != '\0'; i++) if ((varFileName[i] >= 'a') && (varFileName[i] <= 'z')) { varFileName[i] = varFileName[i] - 32; }
20652065

20662066
// Add image information
@@ -4306,8 +4306,8 @@ static Model LoadOBJ(const char *fileName)
43064306
return model;
43074307
}
43084308

4309-
char currentDir[1024] = { 0 };
4310-
strcpy(currentDir, GetWorkingDirectory()); // Save current working directory
4309+
char currentDir[MAX_FILEPATH_LENGTH] = { 0 };
4310+
strncpy(currentDir, GetWorkingDirectory(), MAX_FILEPATH_LENGTH - 1); // Save current working directory
43114311
const char *workingDir = GetDirectoryPath(fileName); // Switch to OBJ directory for material path correctness
43124312
if (CHDIR(workingDir) != 0) TRACELOG(LOG_WARNING, "MODEL: [%s] Failed to change working directory", workingDir);
43134313

@@ -5025,10 +5025,8 @@ static ModelAnimation *LoadModelAnimationsIQM(const char *fileName, int *animCou
50255025
for (unsigned int j = 0; j < iqmHeader->num_poses; j++)
50265026
{
50275027
// If animations and skeleton are in the same file, copy bone names to anim
5028-
if (iqmHeader->num_joints > 0)
5029-
memcpy(animations[a].bones[j].name, fileDataPtr + iqmHeader->ofs_text + joints[j].name, BONE_NAME_LENGTH*sizeof(char));
5030-
else
5031-
strcpy(animations[a].bones[j].name, "ANIMJOINTNAME"); // Default bone name otherwise
5028+
if (iqmHeader->num_joints > 0) memcpy(animations[a].bones[j].name, fileDataPtr + iqmHeader->ofs_text + joints[j].name, BONE_NAME_LENGTH*sizeof(char));
5029+
else memcpy(animations[a].bones[j].name, "ANIMJOINTNAME", 13); // Default bone name otherwise
50325030
animations[a].bones[j].parent = poses[j].parent;
50335031
}
50345032

@@ -6970,7 +6968,7 @@ static Model LoadM3D(const char *fileName)
69706968

69716969
// Add a special "no bone" bone
69726970
model.bones[i].parent = -1;
6973-
strcpy(model.bones[i].name, "NO BONE");
6971+
memcpy(model.bones[i].name, "NO BONE", 7);
69746972
model.bindPose[i].translation.x = 0.0f;
69756973
model.bindPose[i].translation.y = 0.0f;
69766974
model.bindPose[i].translation.z = 0.0f;
@@ -7062,7 +7060,7 @@ static ModelAnimation *LoadModelAnimationsM3D(const char *fileName, int *animCou
70627060

70637061
// A special, never transformed "no bone" bone, used for boneless vertices
70647062
animations[a].bones[i].parent = -1;
7065-
strcpy(animations[a].bones[i].name, "NO BONE");
7063+
memcpy(animations[a].bones[i].name, "NO BONE", 7);
70667064

70677065
// M3D stores frames at arbitrary intervals with sparse skeletons. We need full skeletons at
70687066
// regular intervals, so let the M3D SDK do the heavy lifting and calculate interpolated bones

0 commit comments

Comments
 (0)