The following code is intended to read an incoming packet from a socket and extract one or more headers.
The code performs a check to make sure that the packet does not contain too many headers. However, numHeaders is defined as a signed int, so it could be negative. If the incoming packet specifies a value such as -3, then the malloc calculation will generate a negative number (say, -300 if each header can be a maximum of 100 bytes). When this result is provided to malloc(), it is first converted to a size_t type. This conversion then produces a large value such as 4294966996, which may cause malloc() to fail or to allocate an extremely large amount of memory (CWE-195). With the appropriate negative numbers, an attacker could trick malloc() into using a very small positive number, which then allocates a buffer that is much smaller than expected, potentially leading to a buffer overflow.
BadC
DataPacket *packet;int numHeaders;PacketHeader *headers; sock=AcceptSocketConnection();ReadPacket(packet, sock);numHeaders =packet->headers; if (numHeaders > 100) {ExitError("too many headers!");}headers = malloc(numHeaders * sizeof(PacketHeader);ParsePacketHeaders(packet, headers);
The following code reads a maximum size and performs a sanity check on that size. It then performs a strncpy, assuming it will not exceed the boundaries of the array. While the use of "short s" is forced in this particular example, short int's are frequently used within real-world code, such as code that processes structured data.
This code first exhibits an example of CWE-839, allowing "s" to be a negative number. When the negative short "s" is converted to an unsigned integer, it becomes an extremely large positive integer. When this converted integer is used by strncpy() it will lead to a buffer overflow (CWE-119).
BadC
int GetUntrustedInt () {return(0x0000FFFF);} void main (int argc, char **argv) { char path[256];char *input;int i;short s;unsigned int sz; i = GetUntrustedInt();s = i;/* s is -1 so it passes the safety check - CWE-697 */if (s > 256) {DiePainfully("go away!\n");} /* s is sign-extended and saved in sz */sz = s; /* output: i=65535, s=-1, sz=4294967295 - your mileage may vary */printf("i=%d, s=%d, sz=%u\n", i, s, sz); input = GetUserInput("Enter pathname:"); /* strncpy interprets s as unsigned int, so it's treated as MAX_INT(CWE-195), enabling buffer overflow (CWE-119) */strncpy(path, input, s);path[255] = '\0'; /* don't want CWE-170 */printf("Path is: %s\n", path); }
In the following code, the method retrieves a value from an array at a specific array index location that is given as an input parameter to the method
However, this method only verifies that the given array index is less than the maximum length of the array but does not check for the minimum value (CWE-839). This will allow a negative value to be accepted as the input array index, which will result in reading data before the beginning of the buffer (CWE-127) and may allow access to sensitive memory. The input array index should be checked to verify that is within the maximum and minimum range required for the array (CWE-129). In this example the if statement should be modified to include a minimum range check, as shown below.
BadC
int getValueFromArray(int *array, int len, int index) { int value; // check that the array index is less than the maximum // length of the array if (index < len) { // get the value at the specified index of the array value = array[index]; } // if array index is invalid then output error message // and return value indicating error else {printf("Value is: %d\n", array[index]);value = -1;} return value; }
In the following code, the method retrieves a value from an array at a specific array index location that is given as an input parameter to the method
However, this method only verifies that the given array index is less than the maximum length of the array but does not check for the minimum value (CWE-839). This will allow a negative value to be accepted as the input array index, which will result in reading data before the beginning of the buffer (CWE-127) and may allow access to sensitive memory. The input array index should be checked to verify that is within the maximum and minimum range required for the array (CWE-129). In this example the if statement should be modified to include a minimum range check, as shown below.
GoodC
... // check that the array index is within the correct // range of values for the array if (index >= 0 && index < len) { ...
The following code shows a simple BankAccount class with deposit and withdraw methods.
The withdraw method includes a check to ensure that the withdrawal amount does not exceed the maximum limit allowed, however the method does not check to ensure that the withdrawal amount is greater than a minimum value (CWE-129). Performing a range check on a value that does not include a minimum check can have significant security implications, in this case not including a minimum range check can allow a negative value to be used which would cause the financial application using this class to deposit money into the user account rather than withdrawing. In this example the if statement should the modified to include a minimum range check, as shown below.
BadJava
public class BankAccount { public final int MAXIMUM_WITHDRAWAL_LIMIT = 350; // variable for bank account balance private double accountBalance; // constructor for BankAccount public BankAccount() {accountBalance = 0;} // method to deposit amount into BankAccount public void deposit(double depositAmount) {...} // method to withdraw amount from BankAccount public void withdraw(double withdrawAmount) { if (withdrawAmount < MAXIMUM_WITHDRAWAL_LIMIT) { double newBalance = accountBalance - withdrawAmount;accountBalance = newBalance; }else {System.err.println("Withdrawal amount exceeds the maximum limit allowed, please try again...");...} } // other methods for accessing the BankAccount object ... }
The following code shows a simple BankAccount class with deposit and withdraw methods.
The withdraw method includes a check to ensure that the withdrawal amount does not exceed the maximum limit allowed, however the method does not check to ensure that the withdrawal amount is greater than a minimum value (CWE-129). Performing a range check on a value that does not include a minimum check can have significant security implications, in this case not including a minimum range check can allow a negative value to be used which would cause the financial application using this class to deposit money into the user account rather than withdrawing. In this example the if statement should the modified to include a minimum range check, as shown below.
GoodJava
public class BankAccount { public final int MINIMUM_WITHDRAWAL_LIMIT = 0;public final int MAXIMUM_WITHDRAWAL_LIMIT = 350; ... // method to withdraw amount from BankAccount public void withdraw(double withdrawAmount) { if (withdrawAmount < MAXIMUM_WITHDRAWAL_LIMIT &&withdrawAmount > MINIMUM_WITHDRAWAL_LIMIT) { ...