optimization/8537: Optimizer Removes Code Necessary for Security

[wagnerjd at prodigy dot net]

>Number:         8537
>Category:       optimization
>Synopsis:       Optimizer Removes Code Necessary for Security
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    unassigned
>State:          open
>Class:          wrong-code
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 11 19:26:01 PST 2002
>Closed-Date:
>Last-Modified:
>Originator:     Joseph D. Wagner
>Release:        GCC-3.2
>Organization:
>Environment:
i386
but most likely applicable to all environments
>Description:
When optimizing code for "dead store removal" the optimizing compiler may remove code necessary for security.

For example:

// Beginning of Example Code

#include <string>
using std::string;

#include <memory>

// The specifics of this function are
// not important for demonstrating this bug.
const string getPasswordFromUser() const;

bool isPasswordCorrect() {
	bool isPasswordCorrect = false;
	string Password("password");

	if(Password == getPasswordFromUser()) {
		isPasswordCorrect = true;
	}

	// This line is removed from the optimized code
	// even though it secures the code by wiping
	// the password from memory.
	memset(Password, 0, sizeof(Password));

	return isPasswordCorrect;
}

// End of Example Code

It's considered good practice to remove sensitive data from memory when it's no long needed so that the sensitive data doesn't accidentally end up in the swap file, temp file, memory dump file, etc.

However, in the above example, the optimizing compiler removes the "memset" function as part of "dead store removal" optimization.  The optimizing compiler realizes that "memset" writes to "Password" but "Password" is never again read; hence, it is removed as part of the "dead store removal" optimization.

A programmer could erroneously think that his code is secure, even though the securing code -- "memset" -- is removed from the compiled code.
>How-To-Repeat:
Any code where "memset" is the last function to affect a sensitive variable/data would be affected by this problem.
>Fix:
Two fixes exist from this problem: a workaround and a permanent solution.

WORKAROUND: Read "Password" after it has been wiped from memory.

For example, insert this line after "memset":

// Beginning of Example Code

*(volatile char *)Password = *(volatile char *)Password;

// End of Example Code

This will do nothing, but it will result in reading the "Password" variable, signaling to the optimizing compiler that "memset" should not be "dead store removal" optimized out.

PERMANENT SOLUTION: The WORKAROUND uses both memory and execution time.  A better solution would be to turn off optimization for that part of the code.

However, to the best of my knowledge, GCC does not support altering optimization options on-the-fly though preprocessor statements.  (If it does, will someone please provide me with a link to the appropriate documentation?  I was unable to find any documentation showing that GCC supports this feature.)

For example, altering optimization options on-the-fly through preprocessor statements could be implemented this way:

// Beginning of Example Code

#pragma optimize("-no-dead-code-removal")
memset(Password, 0, sizeof(Password));
#pragma optimize("-dead-code-removal")

// End of Example Code

There's a dozen different ways to accomplish this, but to the best of my understanding and knowledge, the PERMANENT SOLUTION has to be implemented as a new feature (if such a feature doesn't already exist).

>Release-Note:
>Audit-Trail:
>Unformatted: