Revision: 3472 Author: eliot Date: 2015-10-06 11:13:18 -0700 (Tue, 06 Oct 2015) Log Message: ----------- Commit Monty's buffer overflow and error checking fixes to the C library based FilePlugin.
Modified Paths: -------------- trunk/platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c
Property Changed: ---------------- trunk/platforms/Cross/plugins/sqPluginsSCCSVersion.h
Modified: trunk/platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c =================================================================== --- trunk/platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c 2015-10-04 16:51:00 UTC (rev 3471) +++ trunk/platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c 2015-10-06 18:13:18 UTC (rev 3472) @@ -29,8 +29,11 @@
#include <errno.h> #include "sq.h" + #ifndef NO_STD_FILE_SUPPORT + #include "FilePlugin.h" +#include <limits.h> /* for PATH_MAX */
/*** The state of a file is kept in the following structure, @@ -144,33 +147,43 @@ sqFileClose(SQFile *f) { /* Close the given file. */
+ int result; + if (!sqFileValid(f)) return interpreterProxy->success(false); - fclose(getFile(f)); + + result = fclose(getFile(f)); setFile(f, 0); f->sessionID = 0; f->writable = false; setSize(f, 0); f->lastOp = UNCOMMITTED; + + /* + * fclose() can fail for the same reasons fflush() or write() can so + * errors must be checked + */ + if (result != 0) + return interpreterProxy->success(false); + return 1; }
sqInt sqFileDeleteNameSize(char* sqFileName, sqInt sqFileNameSize) { - char cFileName[1000]; + char cFileName[PATH_MAX]; int err;
- if (sqFileNameSize >= 1000) { + if (sqFileNameSize >= sizeof(cFileName)) return interpreterProxy->success(false); - }
/* copy the file name into a null-terminated C string */ interpreterProxy->ioFilenamefromStringofLengthresolveAliases(cFileName, sqFileName, sqFileNameSize, false);
err = remove(cFileName); - if (err) { + if (err) return interpreterProxy->success(false); - } + return 1; }
@@ -218,17 +231,18 @@ Squeak must take care of any line-end character mapping. */
- char cFileName[1001]; + char cFileName[PATH_MAX];
/* don't open an already open file */ if (sqFileValid(f)) return interpreterProxy->success(false);
/* copy the file name into a null-terminated C string */ - if (sqFileNameSize > 1000) { + if (sqFileNameSize >= sizeof(cFileName)) return interpreterProxy->success(false); - } - interpreterProxy->ioFilenamefromStringofLengthresolveAliases(cFileName, sqFileName, sqFileNameSize, true); + /* can fail when alias resolution is enabled */ + if (interpreterProxy->ioFilenamefromStringofLengthresolveAliases(cFileName, sqFileName, sqFileNameSize, true) != 0) + return interpreterProxy->success(false);
if (writeFlag) { /* First try to open an existing file read/write: */ @@ -284,12 +298,12 @@ }
/* - * Fill-in files with handles for stdin, stdout and stderr as available and + * Fill-in files with 3 handles for stdin, stdout and stderr as available and * answer a bit-mask of the availability, 1 corresponding to stdin, 2 to stdout * and 4 to stderr, with 0 on error or unavailablity. */ sqInt -sqFileStdioHandlesInto(SQFile files[3]) +sqFileStdioHandlesInto(SQFile files[]) { /* streams connected to a terminal are supposed to be line-buffered anyway. * And for some reason this has no effect on e.g. Mac OS X. So use @@ -418,22 +432,20 @@
sqInt sqFileRenameOldSizeNewSize(char* oldNameIndex, sqInt oldNameSize, char* newNameIndex, sqInt newNameSize) { - char cOldName[1000], cNewName[1000]; + char cOldName[PATH_MAX], cNewName[PATH_MAX]; int err;
- if ((oldNameSize >= 1000) || (newNameSize >= 1000)) { + if ((oldNameSize >= sizeof(cOldName)) || (newNameSize >= sizeof(cNewName))) return interpreterProxy->success(false); - }
/* copy the file names into null-terminated C strings */ interpreterProxy->ioFilenamefromStringofLengthresolveAliases(cOldName, oldNameIndex, oldNameSize, false); - interpreterProxy->ioFilenamefromStringofLengthresolveAliases(cNewName, newNameIndex, newNameSize, false);
err = rename(cOldName, cNewName); - if (err) { + if (err) return interpreterProxy->success(false); - } + return 1; }
@@ -478,16 +490,26 @@
sqInt sqFileFlush(SQFile *f) { + /* Flush stdio buffers of file */
if (!sqFileValid(f)) return interpreterProxy->success(false); pentry(sqFileFlush); - fflush(getFile(f)); + + /* + * fflush() can fail for the same reasons write() can so errors must be checked but + * sqFileFlush() must support being called on readonly files for historical reasons + * so EBADF is ignored + */ + if (fflush(getFile(f)) != 0 && errno != EBADF) + return interpreterProxy->success(false); + return 1; }
sqInt sqFileSync(SQFile *f) { + /* Flush kernel-level buffers of any written/flushed data to disk */
if (!sqFileValid(f)) return interpreterProxy->success(false); @@ -499,7 +521,6 @@
sqInt sqFileTruncate(SQFile *f,squeakFileOffsetType offset) { - if (!sqFileValid(f)) return interpreterProxy->success(false); if (sqFTruncate(getFile(f), offset)) @@ -508,7 +529,6 @@ return 1; }
- sqInt sqFileValid(SQFile *f) { return (
Property changes on: trunk/platforms/Cross/plugins/sqPluginsSCCSVersion.h ___________________________________________________________________ Modified: checkindate - Sun Oct 4 09:50:05 PDT 2015 + Tue Oct 6 11:12:47 PDT 2015
vm-dev@lists.squeakfoundation.org