From 14863cd3b17e4e8178ae8107c4788d61844f6c96 Mon Sep 17 00:00:00 2001 From: Tamas TEVESZ Date: Tue, 28 Sep 2010 22:25:44 +0200 Subject: [PATCH] WINGs: Bounded string ops Signed-off-by: Tamas TEVESZ --- WINGs/dragsource.c | 2 +- WINGs/findfile.c | 61 +++++++++++++++++++++-------------- WINGs/string.c | 46 ++++++++++++++++++++------- WINGs/userdefaults.c | 35 +++++++++++++++------ WINGs/wapplication.c | 45 ++++++++++++++++---------- WINGs/wbrowser.c | 74 ++++++++++++++++++++++++++++++++----------- WINGs/wcolor.c | 8 +++-- WINGs/wfilepanel.c | 75 +++++++++++++++++++++++++++----------------- WINGs/wfontpanel.c | 6 ++-- WINGs/wtextfield.c | 4 +-- 10 files changed, 240 insertions(+), 116 deletions(-) diff --git a/WINGs/dragsource.c b/WINGs/dragsource.c index c1ea68aa..4b134b08 100644 --- a/WINGs/dragsource.c +++ b/WINGs/dragsource.c @@ -487,7 +487,7 @@ static void registerDescriptionList(WMScreen * scr, WMView * view, WMArray * ope for (i = 0; i < count; i++) { text = WMGetDragOperationItemText(WMGetFromArray(operationArray, i)); - strcpy(textListItem, text); + wstrlcpy(textListItem, text, size); /* to next text offset */ textListItem = &(textListItem[strlen(textListItem) + 1]); diff --git a/WINGs/findfile.c b/WINGs/findfile.c index 6100cca5..70d1e47c 100644 --- a/WINGs/findfile.c +++ b/WINGs/findfile.c @@ -59,7 +59,6 @@ char *wgethomedir() else home = wstrdup(user->pw_dir); -out: return home; } @@ -99,9 +98,9 @@ char *wexpandpath(char *path) path++; if (*path == '/' || *path == 0) { home = wgethomedir(); - if (strlen(home) > PATH_MAX) + if (strlen(home) > PATH_MAX || + wstrlcpy(buffer, home, sizeof(buffer)) >= sizeof(buffer)) goto error; - strcat(buffer, home); } else { int j; j = 0; @@ -113,9 +112,8 @@ char *wexpandpath(char *path) path++; } home = getuserhomedir(buffer2); - if (!home || strlen(home) > PATH_MAX) + if (!home || wstrlcat(buffer, home, sizeof(buffer)) >= sizeof(buffer)) goto error; - strcat(buffer, home); } } @@ -146,17 +144,18 @@ char *wexpandpath(char *path) if ((i += strlen(buffer2) + 2) > PATH_MAX) goto error; buffer[i] = 0; - strcat(buffer, "$("); - strcat(buffer, buffer2); + if (wstrlcat(buffer, "$(", sizeof(buffer)) >= sizeof(buffer) || + wstrlcat(buffer, buffer2, sizeof(buffer)) >= sizeof(buffer)) + goto error; if (*(path-1)==')') { - if (++i > PATH_MAX) + if (++i > PATH_MAX || + wstrlcat(buffer, ")", sizeof(buffer)) >= sizeof(buffer)) goto error; - strcat(buffer, ")"); } } else { - if ((i += strlen(tmp)) > PATH_MAX) + if ((i += strlen(tmp)) > PATH_MAX || + wstrlcat(buffer, tmp, sizeof(buffer)) >= sizeof(buffer)) goto error; - strcat(buffer, tmp); } } else { while (*path != 0 && *path != '/') { @@ -167,14 +166,14 @@ char *wexpandpath(char *path) } tmp = getenv(buffer2); if (!tmp) { - if ((i += strlen(buffer2) + 1) > PATH_MAX) + if ((i += strlen(buffer2) + 1) > PATH_MAX || + wstrlcat(buffer, "$", sizeof(buffer)) >= sizeof(buffer) || + wstrlcat(buffer, buffer2, sizeof(buffer)) >= sizeof(buffer)) goto error; - strcat(buffer, "$"); - strcat(buffer, buffer2); } else { - if ((i += strlen(tmp)) > PATH_MAX) + if ((i += strlen(tmp)) > PATH_MAX || + wstrlcat(buffer, tmp, sizeof(buffer)) >= sizeof(buffer)) goto error; - strcat(buffer, tmp); } } } else { @@ -266,11 +265,20 @@ char *wfindfile(char *paths, char *file) path = wmalloc(len + flen + 2); path = memcpy(path, tmp, len); path[len] = 0; - if (path[len - 1] != '/') - strcat(path, "/"); - strcat(path, file); + if (path[len - 1] != '/' && + wstrlcat(path, "/", len + flen + 2) >= len + flen + 2) { + wfree(path); + return NULL; + } + + if (wstrlcat(path, file, len + flen + 2) >= len + flen + 2) { + wfree(path); + return NULL; + } + fullpath = wexpandpath(path); wfree(path); + if (fullpath) { if (access(fullpath, F_OK) == 0) { return fullpath; @@ -316,8 +324,11 @@ char *wfindfileinlist(char **path_list, char *file) path = wmalloc(len + flen + 2); path = memcpy(path, path_list[i], len); path[len] = 0; - strcat(path, "/"); - strcat(path, file); + if (wstrlcat(path, "/", len + flen + 2) >= len + flen + 2 || + wstrlcat(path, file, len + flen + 2) >= len + flen + 2) { + wfree(path); + return NULL; + } /* expand tilde */ fullpath = wexpandpath(path); wfree(path); @@ -329,6 +340,7 @@ char *wfindfileinlist(char **path_list, char *file) wfree(fullpath); } } + return NULL; } @@ -373,8 +385,11 @@ char *wfindfileinarray(WMPropList * array, char *file) path = wmalloc(len + flen + 2); path = memcpy(path, p, len); path[len] = 0; - strcat(path, "/"); - strcat(path, file); + if (wstrlcat(path, "/", len + flen + 2) >= len + flen + 2 || + wstrlcat(path, file, len + flen + 2) >= len + flen + 2) { + wfree(path); + return NULL; + } /* expand tilde */ fullpath = wexpandpath(path); wfree(path); diff --git a/WINGs/string.c b/WINGs/string.c index c86970f7..d604fffa 100644 --- a/WINGs/string.c +++ b/WINGs/string.c @@ -125,18 +125,31 @@ char *wtokenjoin(char **list, int count) for (i = 0; i < count; i++) { if (list[i] != NULL && list[i][0] != 0) { - if (i > 0) - strcat(flat_string, " "); + if (i > 0 && + wstrlcat(flat_string, " ", j + count + 1) >= j + count + 1) + goto error; + wspace = strpbrk(list[i], " \t"); - if (wspace) - strcat(flat_string, "\""); - strcat(flat_string, list[i]); - if (wspace) - strcat(flat_string, "\""); + + if (wspace && + wstrlcat(flat_string, "\"", j + count + 1) >= j + count + 1) + goto error; + + if (wstrlcat(flat_string, list[i], j + count + 1) >= j + count + 1) + goto error; + + if (wspace && + wstrlcat(flat_string, "\"", j + count + 1) >= j + count + 1) + goto error; } } return flat_string; + +error: + wfree(flat_string); + + return NULL; } void wtokenfree(char **tokens, int count) @@ -185,28 +198,37 @@ char *wstrndup(const char *str, size_t len) char *wstrconcat(char *str1, char *str2) { char *str; + size_t slen; if (!str1) return wstrdup(str2); else if (!str2) return wstrdup(str1); - str = wmalloc(strlen(str1) + strlen(str2) + 1); - strcpy(str, str1); - strcat(str, str2); + slen = strlen(str1) + strlen(str2) + 1; + str = wmalloc(slen); + if (wstrlcpy(str, str1, slen) >= slen || + wstrlcat(str, str2, slen) >= slen) { + wfree(str); + return NULL; + } return str; } char *wstrappend(char *dst, char *src) { + size_t slen; + if (!dst) return wstrdup(src); else if (!src || *src == 0) return dst; - dst = wrealloc(dst, strlen(dst) + strlen(src) + 1); - strcat(dst, src); + slen = strlen(dst) + strlen(src) + 1; + dst = wrealloc(dst, slen); + if (wstrlcat(dst, src, slen) >= slen) + return NULL; return dst; } diff --git a/WINGs/userdefaults.c b/WINGs/userdefaults.c index 3553b361..4eaed824 100644 --- a/WINGs/userdefaults.c +++ b/WINGs/userdefaults.c @@ -57,14 +57,23 @@ char *wusergnusteppath() if (gspath) { pathlen = strlen(gspath) + 4; path = wmalloc(pathlen); - strcpy(path, gspath); + if (wstrlcpy(path, gspath, pathlen) >= pathlen) { + wfree(gspath); + return NULL; + } wfree(gspath); } } else { - pathlen = strlen(wgethomedir()) + 10; + char *h = wgethomedir(); + if (!h) + return NULL; + pathlen = strlen(h) + 8 /* /GNUstep */ + 1; path = wmalloc(pathlen); - strcpy(path, wgethomedir()); - strcat(path, "/GNUstep"); + if (wstrlcpy(path, h, pathlen) >= pathlen || + wstrlcat(path, "/GNUstep", pathlen) >= pathlen) { + wfree(path); + return NULL; + } } return path; @@ -74,13 +83,19 @@ char *wdefaultspathfordomain(char *domain) { char *path; char *gspath; + size_t slen; gspath = wusergnusteppath(); - path = wmalloc(strlen(gspath) + strlen(DEFAULTS_DIR) + strlen(domain) + 4); - strcpy(path, gspath); - strcat(path, DEFAULTS_DIR); - strcat(path, "/"); - strcat(path, domain); + slen = strlen(gspath) + strlen(DEFAULTS_DIR) + strlen(domain) + 4; + path = wmalloc(slen); + + if (wstrlcpy(path, gspath, slen) >= slen || + wstrlcat(path, DEFAULTS_DIR, slen) >= slen || + wstrlcat(path, "/", slen) >= slen || + wstrlcat(path, domain, slen) >= slen) { + wfree(path); + return NULL; + } return path; } @@ -253,7 +268,7 @@ WMUserDefaults *WMGetStandardUserDefaults(void) } } - /* we didn't found the database we are looking for. Go read it. */ + /* we didn't found the database we are looking for. Go read it. XXX: wtf? */ defaults = wmalloc(sizeof(WMUserDefaults)); defaults->defaults = WMCreatePLDictionary(NULL, NULL); defaults->searchList = wmalloc(sizeof(WMPropList *) * 3); diff --git a/WINGs/wapplication.c b/WINGs/wapplication.c index 570dc01a..b7e044d3 100644 --- a/WINGs/wapplication.c +++ b/WINGs/wapplication.c @@ -65,30 +65,41 @@ static char *checkFile(char *path, char *folder, char *ext, char *resource) { char *ret; int extralen; + size_t slen; if (!path || !resource) return NULL; - extralen = (ext ? strlen(ext) : 0) + (folder ? strlen(folder) : 0) + 4; - ret = wmalloc(strlen(path) + strlen(resource) + extralen + 8); - strcpy(ret, path); - if (folder) { - strcat(ret, "/"); - strcat(ret, folder); - } - if (ext) { - strcat(ret, "/"); - strcat(ret, ext); - } - strcat(ret, "/"); - strcat(ret, resource); + extralen = (ext ? strlen(ext) + 1 : 0) + (folder ? strlen(folder) + 1 : 0) + 1; + slen = strlen(path) + strlen(resource) + 1 + extralen; + ret = wmalloc(slen); - if (access(ret, F_OK) != 0) { - wfree(ret); - ret = NULL; - } + if (strlcpy(ret, path, slen) >= slen) + goto error; + + if (folder && + (wstrlcat(ret, "/", slen) >= slen || + wstrlcat(ret, folder, slen) >= slen)) + goto error; + + if (ext && + (wstrlcat(ret, "/", slen) >= slen || + wstrlcat(ret, ext, slen) >= slen)) + goto error; + + if (wstrlcat(ret, "/", slen) >= slen || + wstrlcat(ret, resource, slen) >= slen) + goto error; + + if (access(ret, F_OK) != 0) + goto error; return ret; + +error: + if (ret) + wfree(ret); + return NULL; } char *WMPathForResourceOfType(char *resource, char *ext) diff --git a/WINGs/wbrowser.c b/WINGs/wbrowser.c index 440acb91..745811ad 100644 --- a/WINGs/wbrowser.c +++ b/WINGs/wbrowser.c @@ -689,6 +689,7 @@ char *WMGetBrowserPathToColumn(WMBrowser * bPtr, int column) { int i, size; char *path; + size_t slen; WMListItem *item; if (column >= bPtr->usedColumnCount) @@ -708,24 +709,34 @@ char *WMGetBrowserPathToColumn(WMBrowser * bPtr, int column) } /* get the path */ - path = wmalloc(size + (column + 1) * strlen(bPtr->pathSeparator) + 1); - /* ignore first / */ - *path = 0; + slen = size + (column + 1) * strlen(bPtr->pathSeparator) + 1; + path = wmalloc(slen); + /* ignore first `/' */ for (i = 0; i <= column; i++) { - strcat(path, bPtr->pathSeparator); + if (wstrlcat(path, bPtr->pathSeparator, slen) >= slen) + goto error; + item = WMGetListSelectedItem(bPtr->columns[i]); if (!item) break; - strcat(path, item->text); + + if (wstrlcat(path, item->text, slen) >= slen) + goto error; + } return path; + +error: + wfree(path); + return NULL; } WMArray *WMGetBrowserPaths(WMBrowser * bPtr) { int column, i, k, size, selNo; char *path; + size_t slen; WMListItem *item, *lastItem; WMArray *paths, *items; @@ -760,11 +771,14 @@ WMArray *WMGetBrowserPaths(WMBrowser * bPtr) for (k = 0; k < selNo; k++) { /* get the path */ lastItem = WMGetFromArray(items, k); - path = wmalloc(size + (lastItem != NULL ? strlen(lastItem->text) : 0)); - /* ignore first / */ - *path = 0; + slen = size + (lastItem != NULL ? strlen(lastItem->text) : 0); + path = wmalloc(slen); + /* ignore first `/' */ for (i = 0; i <= column; i++) { - strcat(path, bPtr->pathSeparator); + if (wstrlcat(path, bPtr->pathSeparator, slen) >= slen) { + wfree(path); + return NULL; + } if (i == column) { item = lastItem; } else { @@ -772,7 +786,10 @@ WMArray *WMGetBrowserPaths(WMBrowser * bPtr) } if (!item) break; - strcat(path, item->text); + if (wstrlcat(path, item->text, slen) >= slen) { + wfree(path); + return NULL; + } } WMAddToArray(paths, path); } @@ -1101,27 +1118,48 @@ static void destroyBrowser(WMBrowser * bPtr) static char *createTruncatedString(WMFont * font, char *text, int *textLen, int width) { - int dLen = WMWidthOfString(font, ".", 1); - char *textBuf = (char *)wmalloc((*textLen) + 4); + size_t slen; + int dLen; + char *textBuf; + + dLen = WMWidthOfString(font, ".", 1); + slen = *textLen + 4; + textBuf = wmalloc(slen); if (width >= 3 * dLen) { - int dddLen = 3 * dLen; int tmpTextLen = *textLen; - strcpy(textBuf, text); - while (tmpTextLen && (WMWidthOfString(font, textBuf, tmpTextLen) + dddLen > width)) + if (wstrlcpy(textBuf, text, slen) >= slen) + goto error; + + while (tmpTextLen && (WMWidthOfString(font, textBuf, tmpTextLen) + 3 * dLen > width)) tmpTextLen--; - strcpy(textBuf + tmpTextLen, "..."); + + if (wstrlcpy(textBuf + tmpTextLen, "...", slen) >= slen) + goto error; + *textLen = tmpTextLen + 3; + } else if (width >= 2 * dLen) { - strcpy(textBuf, ".."); + if (wstrlcpy(textBuf, "..", slen) >= slen) + goto error; + *textLen = 2; + } else if (width >= dLen) { - strcpy(textBuf, "."); + if (wstrlcpy(textBuf, ".", slen) >= slen) + goto error; + *textLen = 1; + } else { *textBuf = '\0'; *textLen = 0; } + return textBuf; + +error: + wfree(textBuf); + return NULL; } diff --git a/WINGs/wcolor.c b/WINGs/wcolor.c index 8c74f7cd..cb04e63a 100644 --- a/WINGs/wcolor.c +++ b/WINGs/wcolor.c @@ -314,9 +314,13 @@ unsigned short WMGetColorAlpha(WMColor * color) char *WMGetColorRGBDescription(WMColor * color) { - char *str = wmalloc(32); + char *str = wmalloc(8); - sprintf(str, "#%02x%02x%02x", color->color.red >> 8, color->color.green >> 8, color->color.blue >> 8); + if (snprintf(str, 8, "#%02x%02x%02x", + color->color.red >> 8, color->color.green >> 8, color->color.blue >> 8) >= 8) { + wfree(str); + return NULL; + } return str; } diff --git a/WINGs/wfilepanel.c b/WINGs/wfilepanel.c index cf0af451..5a8aa2cd 100644 --- a/WINGs/wfilepanel.c +++ b/WINGs/wfilepanel.c @@ -1,15 +1,16 @@ -#include "WINGsP.h" -#include "wconfig.h" - #include #include + #include #include #include #include #include +#include "WINGsP.h" +#include "wconfig.h" + #ifndef PATH_MAX #define PATH_MAX 1024 #endif @@ -522,10 +523,13 @@ static void listDirectoryOnColumn(WMFilePanel * panel, int column, char *path) if (strcmp(dentry->d_name, ".") == 0 || strcmp(dentry->d_name, "..") == 0) continue; - strcpy(pbuf, path); - if (strcmp(path, "/") != 0) - strcat(pbuf, "/"); - strcat(pbuf, dentry->d_name); + if (wstrlcpy(pbuf, path, sizeof(pbuf)) >= sizeof(pbuf)) + goto out; + if (strcmp(path, "/") != 0 && + wstrlcat(pbuf, "/", sizeof(pbuf)) >= sizeof(pbuf)) + goto out; + if (wstrlcat(pbuf, dentry->d_name, sizeof(pbuf)) >= sizeof(pbuf)) + goto out; if (stat(pbuf, &stat_buf) != 0) { #ifdef VERBOSE @@ -543,6 +547,7 @@ static void listDirectoryOnColumn(WMFilePanel * panel, int column, char *path) } WMSortBrowserColumnWithComparer(bPtr, column, comparer); +out: closedir(dir); } @@ -617,12 +622,13 @@ static void createDir(WMButton * bPre, WMFilePanel * panel) slen = strlen(dirName) + (directory ? strlen(directory) + 1 /* "/" */ : 0) + 1 /* NULL */; file = wmalloc(slen); - if (directory) { - strncpy(file, directory, slen - 1); - strncat(file, "/", slen - strlen(file)); - } + if (directory && + (wstrlcat(file, directory, slen) >= slen || + wstrlcat(file, "/", slen) >= slen)) + goto out; - strncat(file, dirName, slen - strlen(file)); + if (wstrlcat(file, dirName, slen) >= slen) + goto out; if (mkdir(file, 00777) != 0) { #define __msgbufsize__ 512 @@ -635,6 +641,7 @@ static void createDir(WMButton * bPre, WMFilePanel * panel) WMSetFilePanelDirectory(panel, file); } +out: if (dirName) wfree(dirName); if (directory) @@ -783,29 +790,41 @@ static char *getCurrentFileName(WMFilePanel * panel) { char *path; char *file; - char *tmp; - int len; + char *ret; + size_t slen; path = WMGetBrowserPath(panel->browser); - len = strlen(path); - if (path[len - 1] == '/') { - file = WMGetTextFieldText(panel->fileField); - tmp = wmalloc(strlen(path) + strlen(file) + 8); - if (file[0] != '/') { - strcpy(tmp, path); - strcat(tmp, file); - } else - strcpy(tmp, file); + if (!path) + return NULL; - wfree(file); - wfree(path); - return tmp; - } else { + if (path[strlen(path) -1] != '/') return path; - } + + file = WMGetTextFieldText(panel->fileField); + slen = strlen(path) + strlen(file) + 1; + ret = wmalloc(slen); + + if (*file != '/' && + wstrlcat(ret, path, slen) >= slen) + goto error; + + if (wstrlcat(ret, file, slen) >= slen) + goto error; + + wfree(file); + wfree(path); + return ret; + +error: + wfree(file); + wfree(path); + wfree(ret); + + return NULL; } + static Bool validOpenFile(WMFilePanel * panel) { WMListItem *item; diff --git a/WINGs/wfontpanel.c b/WINGs/wfontpanel.c index 91954b64..53173a71 100644 --- a/WINGs/wfontpanel.c +++ b/WINGs/wfontpanel.c @@ -546,7 +546,7 @@ static void listFamilies(WMScreen * scr, WMFontPanel * panel) WMListItem *item; WM_ITERATE_ARRAY(array, fam, i) { - strcpy(buffer, fam->name); + wstrlcpy(buffer, fam->name, sizeof(buffer)); item = WMAddListItem(panel->famLs, buffer); item->clientData = fam; @@ -628,7 +628,7 @@ static void familyClick(WMWidget * w, void *data) int top = 0; WMListItem *fitem; - strcpy(buffer, face->typeface); + wstrlcpy(buffer, face->typeface, sizeof(buffer)); if (strcasecmp(face->typeface, "Roman") == 0) top = 1; if (strcasecmp(face->typeface, "Regular") == 0) @@ -753,7 +753,7 @@ static void setFontPanelFontName(FontPanel * panel, char *family, char *style, d int top = 0; WMListItem *fitem; - strcpy(buffer, face->typeface); + wstrlcpy(buffer, face->typeface, sizeof(buffer)); if (strcasecmp(face->typeface, "Roman") == 0) top = 1; if (top) diff --git a/WINGs/wtextfield.c b/WINGs/wtextfield.c index a2af3a1f..67cc230d 100644 --- a/WINGs/wtextfield.c +++ b/WINGs/wtextfield.c @@ -396,7 +396,7 @@ void WMInsertTextFieldText(WMTextField * tPtr, char *text, int position) if (position < 0 || position >= tPtr->textLen) { /* append the text at the end */ - strcat(tPtr->text, text); + wstrlcat(tPtr->text, text, tPtr->bufferSize); tPtr->textLen += len; tPtr->cursorPosition += len; incrToFit(tPtr); @@ -465,7 +465,7 @@ void WMSetTextFieldText(WMTextField * tPtr, char *text) tPtr->bufferSize = tPtr->textLen + TEXT_BUFFER_INCR; tPtr->text = wrealloc(tPtr->text, tPtr->bufferSize); } - strcpy(tPtr->text, text); + wstrlcpy(tPtr->text, text, tPtr->bufferSize); } tPtr->cursorPosition = tPtr->selection.position = tPtr->textLen;