finalize() method for ZipFile eventually calls close(), but there is no guarantee as to how long it will take before the finalize() method will be invoked. In a busy environment, this can result in the JVM using up all of its file handles.Example 2: Under normal conditions, the following fix properly closes the file handle after printing out all the zip file entries. But if an exception occurs while iterating through the entries, the zip file handle will not be closed. If this happens often enough, the JVM can still run out of available file handles.
public void printZipContents(String fName) throws ZipException, IOException, SecurityException, IllegalStateException, NoSuchElementException {
ZipFile zf = new ZipFile(fName);
Enumeration<ZipEntry> e = zf.entries();
while (e.hasMoreElements()) {
printFileInfo(e.nextElement());
}
}
public void printZipContents(String fName) throws ZipException, IOException, SecurityException, IllegalStateException, NoSuchElementException {
ZipFile zf = new ZipFile(fName);
Enumeration<ZipEntry> e = zf.entries();
while (e.hasMoreElements()) {
printFileInfo(e.nextElement());
}
zf.close();
}
SafeEvpPKeyHandle, but calls the DangerousAddRef method without a corresponding call to DangerousRelease.Example 2: The following code creates a new instance of
var pkey = NativeMethods.ENGINE_LOAD_SSL_PRIVATE_KEY(...);
var safeEvpHandle = new SafeEvpPKeyHandle(handle: handle, ownsHandle: true);
bool success = false;
try {
safeEvpHandle.DangerousAddRef(ref success);
var handle = safeEvpHandle.DangerousGetHandle();
} catch (ObjectDisposedException ex) {
//...
} finally {
safeEvpHandle.close();
}
SafeEvpPKeyHandle, but calls the DangerousRelease method without a corresponding call to DangerousAddRef.
var pkey = NativeMethods.ENGINE_LOAD_SSL_PRIVATE_KEY(...);
var safeEvpHandle = new SafeEvpPKeyHandle(handle: handle, ownsHandle: true);
bool success = false;
try {
var handle = safeEvpHandle.DangerousGetHandle();
} catch (ObjectDisposedException ex) {
//...
} finally {
safeEvpHandle.DangerousRelease();
safeEvpHandle.close();
}
SafeEvpPKeyHandle, but fails to reliably let the object release the handle during the finalization phase by setting the ownsHandle parameter to false.
var pkey = NativeMethods.ENGINE_LOAD_SSL_PRIVATE_KEY(...);
var safeEvpHandle = new SafeEvpPKeyHandle(handle: handle, ownsHandle: false);
if (safeEvpHandle.IsInvalid) {
...
}
safeEvpHandle.close();
SafeEvpPKeyHandle, but calls the DangerousGetHandle after the handle has been invalidated by SetHandleAsInvalid, which potentially returns a stale handle value.
var pkey = NativeMethods.ENGINE_LOAD_SSL_PRIVATE_KEY(...);
var safeEvpHandle = new SafeEvpPKeyHandle(handle: handle, ownsHandle: true);
...
safeEvpHandle.SetHandleAsInvalid();
...
var handle = safeEvpHandle.DangerousGetHandle();
...
safeEvpHandle.close();
DirectoryEntry object. But if an exception occurs while executing the LDAP query or processing the results, the DirectoryEntry object will not be closed. This will introduce a memory leak in the application, since DirectoryEntry internally uses COM APIs to query the Active Directory server.
...
DirectoryEntry entry = new DirectoryEntry("LDAP://CN=users,DC=fabrikam,DC=com");
DirectorySearcher mySearcher = new DirectorySearcher(entry);
SearchResultCollection result = mySearcher.FindAll();
CheckUsers(result);
mySearcher.Dispose();
entry.Close();
...
...
lo_client = cl_apc_tcp_client_manager=>create( i_host = host
i_port = port
i_frame = lv_frame
i_protocol = protocol
i_ssl_id = ssl_id
i_event_handler = lo_event_handler ).
" initiate the connection setup, successful connect leads to execution of ON_OPEN
lo_client->connect( ).
...
Example 2: Under normal conditions, the following fix properly closes the socket and any associated streams. But if an exception occurs while reading the input or writing the data to screen, the socket object will not be closed. If this happens often enough, the system will run out of sockets and not be able to handle any further connections.
private void echoSocket(String host, int port) throws UnknownHostException, SocketException, IOException
{
Socket sock = new Socket(host, port);
BufferedReader reader = new BufferedReader(new InputStreamReader(sock.getInputStream()));
while ((String socketData = reader.readLine()) != null) {
System.out.println(socketData);
}
}
private void echoSocket(String host, int port) throws UnknownHostException, SocketException, IOException
{
Socket sock = new Socket(host, port);
BufferedReader reader = new BufferedReader(new InputStreamReader(sock.getInputStream()));
while ((String socketData = reader.readLine()) != null) {
System.out.println(socketData);
}
sock.close();
}
Finalize() method for StreamReader eventually calls Close(), but there is no guarantee as to how long it will take before the Finalize() method is invoked. In fact, there is no guarantee that Finalize() will ever be invoked. In a busy environment, this can result in the VM using up all of its available file handles.
private void processFile(string fName) {
StreamWriter sw = new StreamWriter(fName);
string line;
while ((line = sr.ReadLine()) != null)
processLine(line);
}
finalize() method for FileInputStream eventually calls close(), but there is no guarantee as to how long it will take before the finalize() method will be invoked. In a busy environment, this can result in the JVM using up all of its file handles.
private void processFile(String fName) throws FileNotFoundException, IOException {
FileInputStream fis = new FileInputStream(fName);
int sz;
byte[] byteArray = new byte[BLOCK_SIZE];
while ((sz = fis.read(byteArray)) != -1) {
processBytes(byteArray, sz);
}
}
...
CFIndex numBytes;
do {
UInt8 buf[bufferSize];
numBytes = CFReadStreamRead(readStream, buf, sizeof(buf));
if( numBytes > 0 ) {
handleBytes(buf, numBytes);
} else if( numBytes < 0 ) {
CFStreamError error = CFReadStreamGetError(readStream);
reportError(error);
}
} while( numBytes > 0 );
...
def readFile(filename: String): Unit = {
val data = Source.fromFile(fileName).getLines.mkString
// Use the data
}
...
func leak(reading input: InputStream) {
input.open()
let bufferSize = 1024
let buffer = UnsafeMutablePointer<UInt8>.allocate(capacity: bufferSize)
while input.hasBytesAvailable {
let read = input.read(buffer, maxLength: bufferSize)
}
buffer.deallocate(capacity: bufferSize)
}
...
performOperationInCriticalSection(), but fails to release the lock if an exception is thrown in that method.
Object synchronizationObject = new Object ();
System.Threading.Monitor.Enter(synchronizationObject);
performOperationInCriticalSection();
System.Threading.Monitor.Exit(synchronizationObject);
int helper(char* fName)
{
int status;
...
pthread_cond_init (&count_threshold_cv, NULL);
pthread_mutex_init(&count_mutex, NULL);
status = perform_operation();
if (status) {
printf("%s", "cannot perform operation");
return OPERATION_FAIL;
}
pthread_mutex_destroy(&count_mutex);
pthread_cond_destroy(&count_threshold_cv);
return OPERATION_SUCCESS;
}
CALL "CBL_GET_RECORD_LOCK"
USING file-handle
record-offset
record-length
reserved
END-CALL
IF return-code NOT = 0
DISPLAY "Error!"
GOBACK
ELSE
PERFORM write-data
IF ws-status-code NOT = 0
DISPLAY "Error!"
GOBACK
ELSE
DISPLAY "Success!"
END-IF
END-IF
CALL "CBL_FREE_RECORD_LOCK"
USING file-handle
record-offset
record-length
reserved
END-CALL
GOBACK
.
performOperationInCriticalSection(), but fails to release the lock if an exception is thrown in that method.
ReentrantLock myLock = new ReentrantLock();
myLock.lock();
performOperationInCriticalSection();
myLock.unlock();
performOperationInCriticalSection() but never releases it.
os_unfair_lock lock1 = OS_UNFAIR_LOCK_INIT;
os_unfair_lock_lock(&lock1);
performOperationInCriticalSection();
performOperationInCriticalSection() but never releases it.
let lock1 = OSAllocatedUnfairLock()
lock1.lock()
performOperationInCriticalSection();
incomingStream. The Bitmap is manipulated and persisted to the outgoing stream outgoingStream. The Dispose() method of incomingBitmap and outgoingBitmap is never explicitly called.Bitmap.Dispose() when it sees fit. However, the Bitmap object utilizes scarce, unmanaged system resources. The Garbage Collector may fail to call Dispose() before the unmanaged resource pool is depleted.
private void processBitmap(Stream incomingStream, Stream outgoingStream, int thumbnailSize)
{
Bitmap incomingBitmap = (Bitmap)System.Drawing.Image.FromStream(incomingStream);
bool validBitmap = validateBitmap(incomingBitmap);
if (!validBitmap)
throw new ValidationException(incomingBitmap);
Bitmap outgoingBitmap = new Bitmap(incomingBitmap, new Size(thumbnailSize, thumbnailSize));
outgoingBitmap.Save(outgoingStream, ImageFormat.Bmp);
}
Echo. The class declares one native method that uses C to echo commands entered on the console back to the user.
class Echo {
public native void runEcho();
static {
System.loadLibrary("echo");
}
public static void main(String[] args) {
new Echo().runEcho();
}
}
Echo class:
#include <jni.h>
#include "Echo.h" //the java class fromExample 1compiled with javah
#include <stdio.h>
JNIEXPORT void JNICALL
Java_Echo_runEcho(JNIEnv *env, jobject obj)
{
char buf[64];
gets(buf);
printf(buf);
}
gets(), which does not perform any bounds checking on its input.Example 1 could easily be detected through a source code audit of the native method implementation. This may not be practical or possible depending on the availability of the C source code and the way the project is built, but in many cases it may suffice. However, the ability to share objects between Java and native methods expands the potential risk to much more insidious cases where improper data handling in Java may lead to unexpected vulnerabilities in native code or unsafe operations in native code corrupt data structures in Java.Redirect. The class declares one native JavaScript method, which uses JavaScript to change the document location.
import com.google.gwt.user.client.ui.UIObject;
class MyDiv {
...
public static void changeName(final UIObject object, final String name) {
changeName(object.getElement(), url);
}
public static native void changeName(final Element e, final String name) /*-{
$wnd.jQuery(e).html(name);
}-*/;
...
}
private array variable from a public access method. private array variable from a public access method allows the calling code to modify the contents of the array, effectively giving the array public access and contradicting the intentions of the programmer who made it private.private array variable from a public access method.
public final class urlTool extends Applet {
private URL[] urls;
public URL[] getURLs() {
return urls;
}
...
}
public class CustomerServiceApplet extends JApplet
{
public void paint(Graphics g)
{
...
conn = DriverManager.getConnection ("jdbc:mysql://db.example.com/customerDB", "csr", "p4ssw0rd");
...
package level access to the original outer class. More insidiously, since an inner class can access private fields in their enclosing class, once an inner class becomes a peer class in bytecode, the compiler converts private fields accessed by the inner class into protected fields.
public final class urlTool extends Applet {
private final class urlHelper {
...
}
...
}
finalize()method public.super.finalize() inside an implementation of finalize(). In mobile code situations, the otherwise error prone practice of manual garbage collection can become a security threat if an attacker can maliciously invoke one of your finalize() methods because it is declared with public access. If you are using finalize() as it was designed, there is no reason to declare finalize() with anything other than protected access.public finalize() method.
public final class urlTool extends Applet {
public void finalize() {
...
}
...
}
public, final and static.public, final and static is a bug. Because arrays are mutable objects, the final constraint requires that the array object itself be assigned only once, but makes no guarantees about the values of the array elements. Since the array is public, a malicious program can change the values stored in the array. In most situations the array should be made private.public, final and static.
public final class urlTool extends Applet {
public final static URL[] urls;
...
}
public but not final. public member variables in an Applet and in classes used by an Applet should be declared final to prevent an attacker from manipulating or gaining unauthorized access to the internal state of the Applet.public but not final.
public final class urlTool extends Applet {
public URL url;
...
}
Echo. The class declares one native method that uses C to echo commands entered on the console back to the user.
class Echo
{
[DllImport("mylib.dll")]
internal static extern void RunEcho();
static void main(String[] args)
{
RunEcho();
}
}
Echo class:
#include <stdio.h>
void __stdcall RunEcho()
{
char* buf = (char*) malloc(64 * sizeof(char));
gets(buf);
printf(buf);
}
gets(), which does not perform any bounds checking on its input. As well, buf is allocated but not freed and therefore is a memory leak.Example 1 could easily be detected through a source code audit of the native method implementation. This may not be practical or possible depending on the availability of source code and the way the project is built, but in many cases it may suffice. However, the ability to share objects between the managed and native environments expands the potential risk to much more insidious cases where improper data handling in managed code may lead to unexpected vulnerabilities in native code or to unsafe operations in native code corrupting data structures in managed code.
var params:Object = LoaderInfo(this.root.loaderInfo).parameters;
var ctl:String = String(params["ctl"]);
var ao:Worker;
if (ctl == "Add) {
ao = new AddCommand();
} else if (ctl == "Modify") {
ao = new ModifyCommand();
} else {
throw new UnknownActionError();
}
ao.doAction(params);
var params:Object = LoaderInfo(this.root.loaderInfo).parameters;
var ctl:String = String(params["ctl"]);
var ao:Worker;
var cmdClass:Class = getDefinitionByName(ctl + "Command") as Class;
ao = new cmdClass();
ao.doAction(params);
if/else blocks have been entirely eliminated, and it is now possible to add new command types without modifying the command dispatcher.Worker interface. If the command dispatcher is still responsible for access control, then whenever programmers create a new class that implements the Worker interface, they must remember to modify the dispatcher's access control code. If they fail to modify the access control code, then some Worker classes will not have any access control.Worker object responsible for performing the access control check. An example of the re-refactored code is as follows:
var params:Object = LoaderInfo(this.root.loaderInfo).parameters;
var ctl:String = String(params["ctl"]);
var ao:Worker;
var cmdClass:Class = getDefinitionByName(ctl + "Command") as Class;
ao = new cmdClass();
ao.checkAccessControl(params);
ao.doAction(params);
Continuation object could enable attackers to create unexpected control flow paths through the application, potentially bypassing security checks.continuationMethod property, which determines the name of method to be called when receiving a response.
public Object startRequest() {
Continuation con = new Continuation(40);
Map<String,String> params = ApexPages.currentPage().getParameters();
if (params.containsKey('contMethod')) {
con.continuationMethod = params.get('contMethod');
} else {
con.continuationMethod = 'processResponse';
}
HttpRequest req = new HttpRequest();
req.setMethod('GET');
req.setEndpoint(LONG_RUNNING_SERVICE_URL);
this.requestLabel = con.addHttpRequest(req);
return con;
}
continuationMethod property to be set by runtime request parameters, which enables attackers to call any function that matches the name.
...
Dim ctl As String
Dim ao As New Worker()
ctl = Request.Form("ctl")
If (String.Compare(ctl,"Add") = 0) Then
ao.DoAddCommand(Request)
Else If (String.Compare(ctl,"Modify") = 0) Then
ao.DoModifyCommand(Request)
Else
App.EventLog("No Action Found", 4)
End If
...
...
Dim ctl As String
Dim ao As New Worker()
ctl = Request.Form("ctl")
CallByName(ao, ctl, vbMethod, Request)
...
if/else blocks have been entirely eliminated, and it is now possible to add new command types without modifying the command dispatcher.Worker object. If the command dispatcher is responsible for access control, then whenever programmers create a new method in the Worker class, they must remember to modify the dispatcher's access control logic. If this access control logic becomes stale, then some Worker methods will not have any access control.Worker object responsible for performing the access control check. An example of the re-refactored code is as follows:
...
Dim ctl As String
Dim ao As New Worker()
ctl = Request.Form("ctl")
If (ao.checkAccessControl(ctl,Request) = True) Then
CallByName(ao, "Do" & ctl & "Command", vbMethod, Request)
End If
...
clazz.
char* ctl = getenv("ctl");
...
jmethodID mid = GetMethodID(clazz, ctl, sig);
status = CallIntMethod(env, clazz, mid, JAVA_ARGS);
...
Example 2: Similar to previous example, the application uses the
...
func beforeExampleCallback(scope *Scope){
input := os.Args[1]
if input{
scope.CallMethod(input)
}
}
...
reflect package to retrieve the name of a function to be called from a command-line argument.
...
input := os.Args[1]
var worker WokerType
reflect.ValueOf(&worker).MethodByName(input).Call([]reflect.Value{})
...
String ctl = request.getParameter("ctl");
Worker ao = null;
if (ctl.equals("Add")) {
ao = new AddCommand();
} else if (ctl.equals("Modify")) {
ao = new ModifyCommand();
} else {
throw new UnknownActionError();
}
ao.doAction(request);
String ctl = request.getParameter("ctl");
Class cmdClass = Class.forName(ctl + "Command");
Worker ao = (Worker) cmdClass.newInstance();
ao.doAction(request);
if/else blocks have been entirely eliminated, and it is now possible to add new command types without modifying the command dispatcher.Worker interface. If the command dispatcher is still responsible for access control, then whenever programmers create a new class that implements the Worker interface, they must remember to modify the dispatcher's access control code. If they fail to modify the access control code, then some Worker classes will not have any access control.Worker object responsible for performing the access control check. An example of the re-refactored code is as follows:
String ctl = request.getParameter("ctl");
Class cmdClass = Class.forName(ctl + "Command");
Worker ao = (Worker) cmdClass.newInstance();
ao.checkAccessControl(request);
ao.doAction(request);
Worker interface; the default constructor for any object in the system can be invoked. If the object does not implement the Worker interface, a ClassCastException will be thrown before the assignment to ao, but if the constructor performs operations that work in the attacker's favor, the damage will have already been done. Although this scenario is relatively benign in simple applications, in larger applications where complexity grows exponentially it is not unreasonable to assume that an attacker could find a constructor to leverage as part of an attack.performSelector method which could allow them to create unexpected control flow paths through the application, potentially bypassing security checks.UIApplicationDelegate class.
...
- (BOOL)application:(UIApplication *)application openURL:(NSURL *)url
sourceApplication:(NSString *)sourceApplication annotation:(id)annotation {
NSString *query = [url query];
NSString *pathExt = [url pathExtension];
[self performSelector:NSSelectorFromString(pathExt) withObject:query];
...
$ctl = $_GET["ctl"];
$ao = null;
if (ctl->equals("Add")) {
$ao = new AddCommand();
} else if ($ctl.equals("Modify")) {
$ao = new ModifyCommand();
} else {
throw new UnknownActionError();
}
$ao->doAction(request);
$ctl = $_GET["ctl"];
$args = $_GET["args"];
$cmdClass = new ReflectionClass(ctl . "Command");
$ao = $cmdClass->newInstance($args);
$ao->doAction(request);
if/else blocks have been entirely eliminated, and it is now possible to add new command types without modifying the command dispatcher.Worker interface. If the command dispatcher is still responsible for access control, then whenever programmers create a new class that implements the Worker interface, they must remember to modify the dispatcher's access control code. If they fail to modify the access control code, then some Worker classes will not have any access control.Worker object responsible for performing the access control check. An example of the re-refactored code is as follows:
$ctl = $_GET["ctl"];
$args = $_GET["args"];
$cmdClass = new ReflectionClass(ctl . "Command");
$ao = $cmdClass->newInstance($args);
$ao->checkAccessControl(request);
ao->doAction(request);
Worker interface; the default constructor for any object in the system can be invoked. If the object does not implement the Worker interface, a ClassCastException will be thrown before the assignment to $ao, but if the constructor performs operations that work in the attacker's favor, the damage will have already been done. Although this scenario is relatively benign in simple applications, in larger applications where complexity grows exponentially it is not unreasonable to assume that an attacker could find a constructor to leverage as part of an attack.
ctl = req['ctl']
if ctl=='add'
addCommand(req)
elsif ctl=='modify'
modifyCommand(req)
else
raise UnknownCommandError.new
end
ctl = req['ctl']
ctl << "Command"
send(ctl)
if/else blocks have been entirely eliminated, and it is now possible to add new command types without modifying the command dispatcher.define_method(), or may be called via overriding of missing_method(). Auditing and keeping track of these and how access control code is used with these is very difficult, and when considering this would also depend on what other library code is loaded may make this this a near insurmountable task to do correctly in this manner.
def exec(ctl: String) = Action { request =>
val cmdClass = Platform.getClassForName(ctl + "Command")
Worker ao = (Worker) cmdClass.newInstance()
ao.doAction(request)
...
}
if/else blocks have been entirely eliminated, and it is now possible to add new command types without modifying the command dispatcher.Worker interface. If the command dispatcher is still responsible for access control, then whenever programmers create a new class that implements the Worker interface, they must remember to modify the dispatcher's access control code. If they fail to modify the access control code, then some Worker classes will not have any access control.Worker object responsible for performing the access control check. An example of the re-refactored code is as follows:
def exec(ctl: String) = Action { request =>
val cmdClass = Platform.getClassForName(ctl + "Command")
Worker ao = (Worker) cmdClass.newInstance()
ao.checkAccessControl(request);
ao.doAction(request)
...
}
Worker interface; the default constructor for any object in the system can be invoked. If the object does not implement the Worker interface, a ClassCastException will be thrown before the assignment to ao, but if the constructor performs operations that work in the attacker's favor, the damage will have already been done. Although this scenario is relatively benign in simple applications, in larger applications where complexity grows exponentially it is not unreasonable to assume that an attacker could find a constructor to leverage as part of an attack.performSelector method which could allow them to create unexpected control flow paths through the application, potentially bypassing security checks.UIApplicationDelegate class.
func application(app: UIApplication, openURL url: NSURL, options: [String : AnyObject]) -> Bool {
...
let query = url.query
let pathExt = url.pathExtension
let selector = NSSelectorFromString(pathExt!)
performSelector(selector, withObject:query)
...
}
...
Dim ctl As String
Dim ao As new Worker
ctl = Request.Form("ctl")
If String.Compare(ctl,"Add") = 0 Then
ao.DoAddCommand Request
Else If String.Compare(ctl,"Modify") = 0 Then
ao.DoModifyCommand Request
Else
App.EventLog "No Action Found", 4
End If
...
...
Dim ctl As String
Dim ao As Worker
ctl = Request.Form("ctl")
CallByName ao, ctl, vbMethod, Request
...
if/else blocks have been entirely eliminated, and it is now possible to add new command types without modifying the command dispatcher.Worker object. If the command dispatcher is still responsible for access control, then whenever programmers create a new method within the Worker class, they must remember to modify the dispatcher's access control code. If they fail to modify the access control code, then some Worker methods will not have any access control.Worker object responsible for performing the access control check. An example of the re-refactored code is as follows:
...
Dim ctl As String
Dim ao As Worker
ctl = Request.Form("ctl")
If ao.checkAccessControl(ctl,Request) = True Then
CallByName ao, "Do" & ctl & "Command", vbMethod, Request
End If
...
char* ptr = (char*)malloc (SIZE);
...
if (err) {
abrt = 1;
free(ptr);
}
...
if (abrt) {
logError("operation aborted before commit", ptr);
}