Often Misused: File System

Passing an inadequately-sized output buffer to a path manipulation function can result in a buffer overflow.

Windows provides a large number of utility functions that manipulate buffers containing filenames. In most cases, the result is returned in a buffer that is passed in as input. (Usually the filename is modified in place.) Most functions require the buffer to be at least MAX_PATH bytes in length, but you should check the documentation for each function individually. If the buffer is not large enough to store the result of the manipulation, a buffer overflow can occur.

Example 1:

char *createOutputDirectory(char *name) {
	char outputDirectoryName[128];
	if (getCurrentDirectory(128, outputDirectoryName) == 0) {
		return null;
	}
	if (!PathAppend(outputDirectoryName, "output")) {
		return null;
	}
	if (!PathAppend(outputDirectoryName, name)) {
		return null;
	}
	if (SHCreateDirectoryEx(NULL, outputDirectoryName, NULL)
        != ERROR_SUCCESS) {
		return null;
	}
	return StrDup(outputDirectoryName);
}

In this example the function creates a directory named "output\<name>" in the current directory and returns a heap-allocated copy of its name. For most values of the current directory and the name parameter, this function will work properly. However, if the name parameter is particularly long, then the second call to PathAppend() could overflow the outputDirectoryName buffer, which is smaller than MAX_PATH bytes.

The identified call uses methods which follow symbolic links.

Certain identified functions are known to blindly follow symbolic links. When this happens, your application will open, read, or write data to the file that the symbolic link points to instead of the representation of the symbolic link. An attacker may fool the application into writing to alternate or critical system files or provide compromised data to the application.

Example 1: The following code utilizes functions which follow symbolic links:

...
	struct stat output;
	int ret = stat(aFilePath, &output);
	// error handling omitted for this example
	struct timespec accessTime = output.st_atime;
...

The mask specified by the argument umask() is often confused with the argument to chmod().

The umask() man page begins with the false statement:

"umask sets the umask to mask & 0777"

Although this behavior would better align with the usage of chmod(), where the user provided argument specifies the bits to enable on the specified file, the behavior of umask() is in fact opposite: umask() sets the umask to ~mask & 0777.

The umask() man page goes on to describe the correct usage of umask():

"The umask is used to set initial file permissions on a newly-created file. Specifically, permissions in the umask are turned off from the mode argument (so, for example, the common umask default value of 022 results in new files being created with permissions 0666 & ~022 = 0644 = rw-r--r-- in the usual case where the mode is specified as 0666)."

The call uses methods which write to temporary files before writing to the targeted file.

Many APIs will minimize the risk of data loss by completely writing to a temporary file, then copy the complete file to the target destination. Make sure the identified method does not work on files or paths in public or temporary directories as an attacker may replace the temporary file the instant before it is written to the targeted file. This allows the attacker to control the content of files used by the application in public directories.

Example 1: The following code writes the active transactionId to a temporary file in the application Documents directory using a vulnerable method:

...
//get the documents directory:
let documentsPath = NSSearchPathForDirectoriesInDomains(.DocumentDirectory, .UserDomainMask, true)[0]
//make a file name to write the data to using the documents directory:
let fileName = NSString(format:"%@/tmp_activeTrans.txt", documentsPath)
// write data to the file
let transactionId = "TransactionId=12341234"
transactionId.writeToFile(fileName, atomically:true)
...