Auditing Control Flow
| As you learned in Chapter 4, "Application Review Process," control flow refers to the manner in which a processor carries out a certain sequence of instructions. Programming languages have several constructs that enable programmers to branch to different code paths based on a condition, repeat instructions over a number of iterations, and call subroutines (directly or indirectly). These constructs are the basic building blocks of programming languages, and every developer is familiar with them. When auditing code, it's interesting to see that these constructs often have similar security vulnerabilitiesnot because programmers don't know how to implement them, but because the application can enter a specific context that isn't accounted for correctly. In this section, you examine loop and switch statement constructs, which govern internal control flow. External control flow is covered in "Auditing Functions" later in this chapter. For now, you focus on how to audit loops and switch-style branches and learn some guidelines on what to look for when evaluating their proper use. Looping ConstructsLooping constructs are extremely common and used in every component of application processing, whether it's initializing structures, processing input, interacting with the file system, or deallocating memory. This section focuses on data-processing loops, which are used to interpret user-supplied data and construct some form of output based on the data. This output can range from elements extracted from user input to data derived from the input. These types of loops pose the most immediate security threat to an application. A loop can be constructed incorrectly in a number of ways so that it causes a read or write outside the bounds of the provided data buffers. The following common errors can cause loops to behave in a manner that contradicts the programmer's intentions:
Any of these conditions can have potentially disastrous consequences for application security, particularly if the loop performs writes to memory in some way. As discussed in Chapter 5, "Memory Corruption," writes stand the most chance of being destructive to other variables or program state information and, consequently, leading to an exploitable situation. Terminating ConditionsApplication developers are often required to construct loops for processing user-malleable data. These loops must parse and extract data fields, search for occurrences of specific data elements, or store parts of data to a specific destination, such as another memory location or a file. When a loop performs a data copy, it is necessary to verify whether the copy is performed in a safe mannerthat is, there's no way the loop can read or write outside the boundaries of objects being operated on. Typically, loops that perform these kinds of copies have multiple terminating conditions, which are checks that can cause the loop to exit. A loop might have a terminating condition that checks for successful completion of the copy as well as several terminating conditions that attempt to account for erroneous conditions that might occur during processing. If the set of terminating conditions in a loop don't adequately account for all possible error conditions, or the implementation of the checks is incorrect, the program might be susceptible to compromise in one form or another. When dealing with length calculations, two main problems could occur:
The first problem is fairly easy; no size check is done on input or output data, so if attackers can supply more data than has been allocated for the destination buffer, they can trigger a buffer overflow condition and compromise the application. Listing 7-15 shows a simple example. Listing 7-15. Simple Nonterminating Buffer Overflow Loop
The code in Listing 7-15 essentially performs the same task as a strcpy() routine: It copies data from src into dst until it encounters a NUL byte. These types of loops are usually quite easy to spot when auditing code and appear quite often in major applications. A notable example is one in the Distributed Component Object Model (DCOM) Object Activation RPC interface in Windows operating systems. This interface has a tightly contained loop in the GetMachineName() function. Listing 7-16 shows approximated C code based on the assembly code. Listing 7-16. MS-RPC DCOM Buffer Overflow Listing
As you can see, this buffer overflow is similar to Listing 7-15 and is performing a potentially dangerous copy. Sometimes, however, when you read complex functions containing nested loops, these types of suspect loop constructs can be difficult to spot. Often it's hard to verify whether they present a potential vulnerability. Listing 7-17 from NTPD, the network time protocol (NTP) daemon, demonstrates a more complicated copying loop. Listing 7-17. NTPD Buffer Overflow Example
The code in Listing 7-17 is processing an NTP control packet. It's vulnerable to a buffer overflow, as the destination buffer (pointed to by tp) isn't verified to be large enough to hold input data supplied by users. A lot of the surrounding code is included in the listing to demonstrate how complex code can sometimes make auditing difficult. You might need to make several passes before you understand what the preceding function is doing and whether the while loop construct that first appears to be vulnerable can be reached with values that trigger the vulnerability. Auditing Tip When data copies in loops are performed with no size validation, check every code path leading to the dangerous loop and determine whether it can be reached in such a way that the source buffer can be larger than the destination buffer. The second problem in dealing with length calculations, as mentioned at the beginning of this section, is a size check done incorrectly. In recent years, application developers have started to be more careful with looping constructs by putting in length validation as one of the loop's terminating conditions. However, the check is sometimes implemented incorrectly, leaving the application vulnerable to compromise. The first common mistake is an off-by-one error (discussed in Chapter 5). This vulnerability most commonly occurs in string processing, as in the following example: for(i = 0; src[i] && i < sizeof(dst); i++) dst[i] = src[i]; dst[i] = '\0'; Technically, the loop is not at fault here. It writes data to the destination buffer, and it doesn't write outside the bounds of the dst buffer. The statement immediately following it, however, could write one byte past the end of the array. This occurs when the loop terminates because i is equal to the size of the destination buffer buf. In this case, the statement dst[i] = '\0' is then equivalent to dst[sizeof (dst)] = '\0', which writes the NUL one byte past buf's allocated space. This bug is commonly associated with loops of this nature. The previous vulnerability brings up the next interesting behavior to look out for: Occasionally, when loops terminate in an unexpected fashion, variables can be left in an inconsistent state. It's important to determine which variables are influenced by a looping construct and whether any potential exit conditions (typically boundary conditions signifying an error of some sort) leave one of these variables in an inconsistent state. Naturally, this determination is relevant only if the variables are used after the loop completes. Listing 7-18 shows some code taken from mod_php that reads POST data from users. Listing 7-18. Apache mod_php Nonterminating Buffer Vulnerability
The sapi_read_standard_form_data function is expected to fill the global buffer post_data and place a NUL byte at the end of the buffer. However, it doesn't in one case: If more than post_max_size data is supplied, a warning is generated and the function returns. Because this function is a void function and doesn't return a value, the function's caller doesn't know an error has occurred and continues processing unaware. Note that in some circumstances, the php_error_docref() function can cause the process to exit, depending on the second argument; however, in this case the function just generates a warning. In normal circumstances, a bug like this would present potential exploitation opportunities by causing a pointer to increment outside the bounds of the post_data variable. However, in this case, the allocator doesn't let you supply post_max_size (8 MB) bytes in a request because there's a memory limit of 8MB per request (although both the memory allocation maximum data limit and post_max_size can be configured). Auditing Tip Mark all the conditions for exiting a loop as well as all variables manipulated by the loop. Determine whether any conditions exist in which variables are left in an inconsistent state. Pay attention to places where the loop is terminated because of an unexpected error, as these situations are more likely to leave variables in an inconsistent state. Another off-by-one error occurs when a variable is incorrectly checked to ensure that it's in certain boundaries before it's incremented and used. Listing 7-19, which is code from the mod_rewrite Apache module, demonstrates this error. Listing 7-19. Apache 1.3.29/2.X mod_rewrite Off-by-One Vulnerability
As you can see, the c variable is used to ensure that only five pointers are stored in the array of pointers, token. However, after the size check is performed, the c variable is incremented and used, which means the check should read (*cp && c<4). If an attacker provides input to the loop so that c is equal to four, the input passes the length check but causes the program to write a pointer into token[5], which is outside the allocated space for token. This error can lead to an exploitable condition because the attacker writes a pointer to user-controlled data outside the bounds of the token variable. Loops that can write multiple data elements in a single iteration might also be vulnerable to incorrect size checks. Several vulnerabilities in the past happened because of character escaping or expansion that weren't adequately taken into account by the loop's size checking. The Dutch researcher, Scrippie, found a notable bug in OpenBSD 2.8. The code in Listing 7-20, which is taken from OpenBSD's 2.8 ftp daemon, copies data into a destination buffer, escaping double-quote characters (") as it encounters them. Listing 7-20. OpenBSD ftp Off-by-One Vulnerability
The problem in Listing 7-20 is that the i variable can be incremented twice in a single iteration of the loop. Therefore, if a double-quote character is encountered in the source string at location (sizeof(npath)-2), i is incremented twice to hold the value (sizeof(npath)), and the statement immediately following the loop writes a zero byte out of bounds. This code ended up being an exploitable off-by-one vulnerability. Finally, a loop's size check could be invalid because of a type conversion, an arithmetic boundary condition, operator misuse, or pointer arithmetic error. These issues were discussed in Chapter 6. Posttest Versus Pretest LoopsWhen writing program loops, developers can decide whether to use a posttest or a pretest control structure. A posttest loop tests the loop termination condition at the end of each iteration of the loop; a pretest loop tests the condition before each iteration of the loop. In C, posttest and pretest loops can be distinguished easily; a posttest loop uses the do {...} while() construct, and pretest loops use for( ;; ) {...} or while() {...}. Pretest loops tend to be used primarily; posttest loops are used in some situations out of necessity or for personal preference. When encountering loops, code auditors must determine the implications of the developer's choice of loop form and whether that choice could have negative consequences for the code. Posttest loops should be used when the body of the loop always needs to be performed at least one time. As an auditor, you should look for potential situations where execution of the loop body can lead to an unexpected condition. One thing to look out for is the conditional form of the loop performing a sanity check that should be done before the loop is entered. Consider the example in Listing 7-21, which uses a posttest loop to do some simple string processing. Listing 7-21. Postincrement Loop Vulnerability
In this code, if the data supplied is an empty string (a string containing the NUL character only), the pointer cp is incremented past the string's intended bounds. The loop continues processing undefined data, potentially resulting in a memory corruption or information leak vulnerability of some kind. The programmer should have checked whether cp points to a NUL character before entering the loop, so this loop should have been written in pretest form. Likewise, a programmer can use a pretest loop when a posttest format would be more appropriate and, consequently, create an exploitable situation. If the code following a loop expects that the loop body has run at least once, an attacker might be able to intentionally skip the loop entirely and create an exploitable condition. Take a look at the code in Listing 7-22. Listing 7-22. Pretest Loop Vulnerability
In this example, the code following the loop assumes that the token_array array has been allocated, which can happen only if the loop runs at least once. If the first call to parse_element() returns NULL, the loop isn't entered, token_array is never allocated, and the bolded code causes a NULL pointer dereference, resulting in a potential crash. Punctuation ErrorsAs discussed in Chapter 6, typographical errors can lead to situations that have security-relevant consequences. Occasionally, developers make the mistake of inserting superfluous language punctuation where they shouldn't, and this mistake often results in the loop not doing what was intended. Take a look at a simple example: for(i = 0; i < sizeof(dest) && *src != ' '; i++, src++); dest[i] = *src; if(i == sizeof(dest)) i--; dest[i] = '\0'; The for loop in this code is supposed to be copying data into the dest array; however, the programmer made a slight error: a semicolon at the end of the line with the for loop. Therefore, the loop doesn't actually copy anything, and what should be the loop body always runs once after the counter is incremented past the array bounds. This error means you could potentially write a byte to dest[sizeof(dest)], which would be one byte out of bounds. Naturally, these errors aren't that common because they usually break the program's functionality and, therefore, get caught during testing or development. Simple testing of the code in the previous example would probably show the programmer that any subsequent processing of dest seems to have a problem because the loop doesn't copy any data into dest as it's supposed to. However, these errors do occur from time to time in ways that don't affect the program's functionality, or they occur in error-handling or debugging code that hasn't been tested. As discussed in Chapter 6, reviewers should always be on the lookout for these minor punctuation errors. Flow Transfer StatementsProgramming languages usually provide control flow statements that developers can use to redirect execution in very direct ways. Loops typically have a mechanism by which a programmer can immediately terminate a loop or advance a loop to its next iteration. Switch-style statements have keywords for denoting a case body and a mechanism for breaking out of a case body. Some languages provide goto and longjmp style statements, which can allow arbitrary control flow transfers within a function or across function boundaries. Occasionally, application developers misuse these control flow statements in ways that can have security-relevant consequences because these keywords are often overloaded. In C, the break statement is used to break out of a switch statement and to terminate a loop. The dual use of this statement can lead to several potential mistakes. Application developers might assume that a break statement can break out of any nested block and use it in an incorrect place. Or they might assume the statement breaks out of all surrounding loops instead of just the most immediate loop. Another problem is using a continue statement inside a switch statement to restart the switch comparison. Experienced programmers wouldn't consciously make these kinds of mistakes, but they can remain in code if they're caused by accidental editing mistakes, for example, and aren't immediately apparent when using the application. For these mistakes to remain in the code, however, they need to appear correct enough that a casual review wouldn't raise any red flags. A vulnerability of this nature resulted in the much-publicized AT&T phone network outage of 1990. The programmer mistakenly used a break statement to break out of an if code block nested inside a switch statement. As a result, the switch block was unintentionally broken out of instead. Switch StatementsWhen dealing with suspect control flow, switch statements have a few unique considerations. A common pitfall that developers fall into when using switch statements is to forget the break statement at the end of each case clause. This error can result in code being executed unintentionally when the erroneous case clause runs. Take a look at Listing 7-23. Listing 7-23. Break Statement Omission Vulnerability
This code makes a mistake when dealing with the newline ('\n') character. The break statement is missing, so every newline character causes a space to be written to the destination buffer, followed by a newline character. This happens because the default case clause runs every time the '\n' case is executed. This error results in the code writing more characters into the output buffer than was anticipated. Note In some circumstances, programmers intend to leave the break statement out and often leave a comment (such as /* FALLTHROUGH */) indicating that the omission of the break statement is intentional. When reviewing code containing switch statements, auditors should also determine whether there's any unaccounted-for case. Occasionally, switch statements lacking a default case can be made to effectively run nothing when there isn't a case expression matching the result of the expression in the switch statement. This error is often an oversight on the developer's part and can lead to unpredictable or undesirable results. Listing 7-24 shows an example. Listing 7-24. Default Switch Case Omission Vulnerability
Listing 7-24 initializes an object based on the supplied type variable. The init_object() function makes the assumption that the type variable supplied is OBJ_STR, OBJ_INT, or OBJ_BOOL. If attackers could supply a value that wasn't any of these three values, this function wouldn't correctly initialize the allocated object structure, which means uninitialized memory could be treated as pointers at a later point in the program. |
EAN: 2147483647
Pages: 194