This is an almost entirely direct copy/paste from my personal notes repo and was last updated approx 2020

Code Review Process

My note words

  • OOBW - Out Of Bounds Write
  • OOBR - ^ Read
  • ACD - Attacker Controlled Data, a source of data
  • dj - a comment by me
  • djtodo - come back to this
  • djvuln - a vuln to be PoC’d

C Background

Terms

  • unspecified behavior: use of unspecified value, or other behavior where the standard provides two or more possibilities and imposes no further requirement on which is chosen in any instance
  • implementation-defined behavior: unspecified behavior where each implementation documents how the choice is made.
  • undefined behavior: behavior, upon use of a nonportable or erronoeus program construct or of erroneous data, for which the standard imposes no requirements

char

  • Guaranteed to be 1 byte [not necessarily 8 bits, but probably].
  • Can be unsigned char, signed char, or char which C std says is IMPLDEF.

int

  • short int, int, long int, long long int (last one is new to C99)
  • short typically 16 bits on 32 bit platform
  • int & long usually 32 bits on 32 bit
  • long long usually 64 bits on 32 bit processor
  • 64 bit systems: short, int, long, long long, ptr
  • 64 bit Windows: 16, 32, 32, 64, 64
  • 64 bit Unix: 16, 32, 64, 64, 64

float

  • float, double, long double
  • float _Complex, double _Complex, long double _Complex

bit field

  • for example: unsigned int id:4; is 4 bits

Literal numbers:

  • int has no decimal, float does
  • float f = (1/2) + (1/2); //f is 0

Number representations:

C std actually allows for three representations:

  • Sign and magnitude. has two zeros.
  • Ones complement: invert all bits to negate. has two zeros
  • Two’s complement: invert all bits and add 1 to negate.

Endianness:

  • Big endian: MSB comes first, then lesser
  • Little endian: LSB comes first. Intel == LE

Type Conversions

Some definitions:

  • Simple conversion: explicit casts, assignments, function args/return values etc.
  • Integer promotion: conversion to int for certain operators that require int, or usual arithmetic conversion.
  • Usual arithmetic conversions: implicit conversions to find a common real type (arithmetic, relational, equality, bitwise, ternary)
  • Value-Preserving: The new type can represent all values possible in the old type.
  • Value-Changing: New type can’t represent all. IE int–>unsigned int
  • Widening: converting from narrow to wider. Unsigned will use a zero extension and signed will use a sign extension (propagate sign bit).
  • Truncation: Converting to a narrower type simply truncates higher bits.

Rank: long long int –> long int –> int –> short –> char –> _Bool

(unsigned varieties have same rank as signed varieties. there are 3 char types, all equal)

Where do integer promotions to int/unsigned int occur?

  • + and - and ~ operators
  • Bitwise shift operators
  • Switch statements

Usual Arithmetic Conversions

Steps:

  1. Is either type a float? if so, promote to largest float precision in arithmetic
  2. Integer promotions: both operands must be minimum of int/unsigned int precision
  3. If operands are now both the same type, we’re done
  4. If operands have the same sign but different widths, widen the narrower one so they’re equal. done.
  5. If there’s an unsigned type wider or same as a signed type, signed type is converted to unsigned.
  6. If there’s a wider signed type and narrow unsigned, generally unsigned can be converted to wider signed type and is value preserving.
  7. Page 241 of TOASSA, todo.

Where are these used?

  • addition, subtraction, multiply, divide
  • relational and equality operators
  • bitwise binary operators
  • question mark operator

Pointers

  • const char* is a pointer to a const char
  • char const* is a pointer to a const char
  • char* const is a constant pointer to a char
  • const char* const is a constant pointer to a constant char

Storage Classifiers / Type Qualifiers

  • inline (C99) the compiler should inline func, instead of call/ret
  • static function limits visibility to that file
  • static variable initialized once, preserves value over function calls. Automatically initialized to a value.
  • volatile no caching allowed, cannot assume value is unchanged (IE Memory Mapped IO)
    • caveat: it will guarantee access to that variable, but not against re-ordering of other statements.
  • const: will not change. generally prefered over #define as allows scoping rules and checks syntax prior to C Preprocessor.

Operator Order of Precedence

Vulns

High-level categories

  • Memory Corruption
  • Crypto
  • Design/Logic
  • PrivEsc
  • Dangerous File/IO or racing

Files

  • When a zero-length (ie no bytes written) file is closed, it’s IMPLDEF whether the file is created on disk
  • If a program exits using anything other than exit() (ie abort()), buffered data may or may not be written, and the file may or may not be closed properly. Linux ensures it always is.
  • fopen()/fclose() are C, use a FILE *, open()/close() are POSIX, use a file descriptor int
  • All the standard ../ directory traversal tricks
    • And if there’s a sanitizer, does it do it correctly? can you craft malicious payloads by using the sanitizer to modify your input string? 
      path = replace(path, "../", "");
// can bypass by inputting "....//"
  • A symlink is a file which contains a path to another file
  • A hardlink is a reference to the file, file is only deleted when every link has been rm‘d and has 0 references to it.
  • If a program checks for symlinks, can you just hardlink (which is essentially a regular file)
  • Hardlinking a file robs the owner from being able to free that disk space up
    • This is one of the reasons it’s recommended to have different partitions for user and system
  • Both fopen() and POSIX open() will create a file if it doesn’t exist, or open existing if it does
    • O_EXCL and O_CREAT passed into open() will only open if the file does not exist.
  • race conditions: TOCTOU
  • Embedded NUL when going from higher-level language to OS. TODO: Tinker with some java/python examples
Dangerous Locations
  • Anything user-supplied: audit heavily
  • New files and directories: can an attacker manipulate this creation?
  • Temporary & public directories: /tmp/, /var/tmp/, etc
  • Files controlled by others: IE dropping privs and writing as the UID
int main(int argc, char **argv) {
    char *file = argv[1];
    int fd; struct stat statbuf;
    stat(file, &statbuf);
    if (S_ISLINK(statbuf.st_mode)) {
        bailout(“symbolic link”);
    }
    else if (statbuf.st_uid != getuid) {
        bailout(“you don’t own the file”);
    }
    fd = open(file, O_RDWR);
    write(fd, argv[2], strlen(argv[2]));
}
  • both stat() and access() often indicate a race condition: they take a filename of an unopened file. What you actually want to do is open the file (resolve inode), then use fstat to check metadata about the opened file. Prevents race conditions.
  • Directories are a type of file which contain directory entries. Write permissions on a directory enable you to “write” to the directory file and delete or rename files, irrespective of the file itselfs permissions.
  • using open() with O_CREAT but not O_EXCL could lead to a process opening an existing file (or more dangerously, a link) when it thinks it’s creating a new file.
  • A process that relies upon file permissions is relying on all parent directory permissions as well; otherwise an attacker can just rename a higher-up directory and potentially provide their own.
  • truncation: snprintf(path, 28 "/root/.ssh/%s.pub", user_controllable) and use ./././././././././id_rsa to steal private key
Race & TOCTOU
  • mktemp() is almost always raceable, because file path is predictable
  • so are tmpnam() and tempnam()
  • mkstemp() does it correctly; unless a program later opens the file by pathname, re-opening the whole vulnerability.
  • TAOSSA:544 details just how damn hard it is to open a file in a non-racy way
functions
  • fgets()
    • Returns when input contains EOF or \n; input can contain NUL!
    • Returns NULL & buf is undefined upon EOF/\n, you must check return value. buf could be huuuge.
  • fscanf()
    • You must check the return value and ensure it’s equal to the number of items you expected to parse
    • Also be careful of BoF’s in scanf() family of functions
  • Some functions promise to delete a symlink if it exists. None of them however, will delete a symlinked directory in the path, so none are safe from that type of symlink attack

Processes

General
  • After a fork(), the exact same kernel-based open file structure is shared
    • an lseek() from child will change the parents location in file and vice versa
  • execlp(), execvp(), and execvpe() all obey PATH env var if path argument doesn’t contain /
    • You could control what is actually executed
  • any fork() or execve()-derivative will result in the child process having access to the parent processes file descriptors; unless they were opened with FD_CLOEXEC - close fd on exec() call
  • Does a program make assumptions about stdin, stdout, stderr file descriptors?
    • fprintf(stderr, "Error: %s"\n");, if someone has managed to close fd 2 and then cause the program to open /etc/passwd, it will b e allocated fd2 and that error will be written to a sensitive file
rlimits
  • A soft limit can be moved up & down by a process
  • A hard limit can only be lowered by a program
  • the program can not go past either of these limits
  • the CLI tool ulimit uses setrlimit() syscall under the hood
  • limits include:
    • various memory limits
    • open files
    • maximum CPU usage
    • maximum size of files written
    • size of core dump files
  • Lots of tricks here (TAOSSA Chapt 10)
    • use RLIMIT_FSIZE to cut off a “username:pass_hash” write to leave an empty hash

environment

  • setenv() will copy your data into a new environment variable it allocated
  • putenv() will just have the environ array use your pointer: no copy occurs. It’s an error to use an automatic variable
    • also, putenv behavior related to whether you use a = char is unreliable
  • environment variables worth fucking with:
    • HOME
    • PATH
    • ENV
    • IFS
    • SHELL
    • EDITOR
    • LD_PRELOAD
    • LD_LIBRARY_PATH

pointer arithmetic

*ptr+1 if ptr points to a struct, it will be *ptr+sizeof(struct) not *void+1

floats

  • floats can be NaN, Inf, (in both -ve and +ve) two different representations of same value etc
  • floats do allow div-by-zero to occur at runtime, resulting in NaN, under clang.

int negation

negate int of 0x80000000 (1«31) doesn’t change the value, for 2’s compliment.

divide-by-zero

  • Undefined according to C11 draft.

Function Pointers

  • Anywhere functions are passed around as values, look for an opportunity for attacker to control what is called. Example, CVE-2018-10933 by Peter Winter-Smith: 
    // ACD from packet used to index array of callbacks; can call any func. Bypass Auth!
r=cb->callbacks[type - cb->start](session,type,session->in_buffer,cb->user);

UAF

Code path allows for use of an object member func or variable after freeing of that object (UAF)

Look through all possible code paths, is there any chance that an object can be free’d and a pointer somewhere is retained to it?

Looping

  • loop break conditions.. do they always break?
  • User controllable increment value in loop (DoS) set to zero for(i=0;i<len;i += USER_CONTROLLABLE){}
  • Loop over data could jump past condition for(i=0; i!=10; i += 11)
  • The following is true that it won’t exit: char i; unsigned char j=200; for(i=0; i<j; i++){
    • because i will wrap at 128 to -1, and both will be converted to int which preserves all their values.

Goto

  • variable sync
  • free
  • leak
  • uninit
  • (all control flow issues)

Switch

  • Missing default statement
  • Missing break (fall-through)

Migration/32 to 64 issues

  • on 64-bit, you cannot store a pointer or ptrdiff_t in an int, or it can truncate
  • comparison between long and unsigned int:
    • ILP32: long is 32, unsigned int is 32, so long is cast to unsigned and unsigned comparison occurs
    • LP64: long is 64, unsigned int is 32, so unsigned int can be represented in long, so signed comparison occurs
    • LLP64, long is 32, unsigned int is 32, so same as ILP32 unsigned comparison occurs
  • Any assumption of a long being 32 bit, such as atol() being assigned to an int: could truncate.
  • functions which return size_t should not be assigned to (unsigned) int, because on LP/LLP64, size_t > int
    • such as: strlen() where a > 4gb string will truncate. or a > 2gb file will create a -ve
  • The following is fine on ILP32, but on LP64, len can be set to something larger than i can ever reach, resulting in an endless loop.
unsigned int i;
size_t len = attacker_controlled(); 
char *buf = malloc(len);
for(i = 0; i < len; i++) { 
    *buf++ = get_next_byte();
}

Sign Extension

  • whenever a signed type is widened, sign extension will occur
  • The MSB will be propogated
  • EVEN IF the wider type is an unsigned type
  • pro-top: to cut through compiler/typedef/etc BS.. search assembly for movsx instructions.
int a = attacker_controlled()           // can input -1
p = malloc((unsigned int)a);            // promoted to int (sign extended), 
                                        // then cast to unsigned 0xffffffff
memcpy(p,src,a);                        // promoted to long (64 on LP64), then cast to 
                                        // unsigned long 0xffffffffffffffff
  • TODO: generate asm listings for ARM/AVR/MIPS/x86 of instructions that identify sign extension

Implicit Coercion

  • Implicit coersion: Ex. if(size_t < char) the char will be promoted to size_t 
    char x;
unsigned char ct;
for (x=0; x < ct; x++ ) {
  // both are promoted to int, but wrapping maintains consistency
  // ct can be set to 200 and x will never reach it
  // for increment, byte [rbp+var_5] is mov'd into al, so it wraps as a byte
}

Promotion

  • Usual arithmetic promotions: 
    signed char cres, c1, c2, c3;
c1 = 100;
c2 = 3;
c3 = 4;
cres = c1 * c2 / c3;  //promotion to int avoids wrap of signed char
  • Rankings: Long Long Int, Long Int, Int, Short Int, Char, _Bool
  • Both signed or unsigned: narrower variable promoted to wider
  • One signed, one unsigned: if signed can represent all values of unsigned, unsigned is promoted to signed.
  • One signed, one unsigned: unless the signed is higher rank, the signed is promoted to unsigned. 
    unsigned int i = 200;
int j = -21;
if (j > i) { 
  /* evaluates to true because j is promoted to unsigned int and goes large */

Simple Conversion

  • Three conversions can occur in a return:
    • The type that is returned, ie int i; ...; return i;
    • The type in the function prototype, ie char doThing()
    • The type it is assigned to, ie uint8 x = doThing()

Truncation

  • truncation; if a wider type is copied into a narrower type, truncation will occur.
  • IE short–>char or int–>short
  • occurs during:
    • assignment
    • typecast
    • function call
  • be on the lookout for any char or short values that are likely to be used to store lengths or somehow receive a value from a larger type.

Comparison

  • Remember the integer promotions and usual arithmetic promotions during any arithmetic
short length = get_network_short(sockfd);

if (length - sizeof(short) <= 0 || length > MAX_SIZE) {
  // length is cast to `size_t` in the first condition, and cannot be negative
  // a value of -1 will pass the second condition 
}
  • A big gotcha / thing to look for: if (unsignedVar < 0) { // impossible
  • The above will at least throw an error with -Wall, whereas if (unsignedVar <= 0) will not

Macros

The following is obviously wrong:

#define ADD_TWO(x) x+2

printf("%d" 10 * ADD_TWO(2));
// answer will be 22, not 40

So is the following:

#define THING(x) g(x); h(x)

if (a==1) 
    THING(z);
// h(x) is always executed, not just when `if` statement is true

And

#define DOUBLE(x) ((x)+(x))   //parens to be safe

DOUBLE(j++);
  • expand macros/define in-line in order to see if they mess up order of precedence. macros should probably be bracketed. can test with precompiler IE gcc -E file.c

Signals

  • Signals can introduce race conditions into a single-threaded application
  • Signal handlers should do very simple things like set a flag. Most syscalls are not reentrant.
    • For example, syslog() calls malloc() and thus is not reentrant or safe
    • free() is not safe
    • nor printf()
    • etc
  • Look for any callpath where there is for example, a free(); if you can send 2x different signals, you can possibly have a double free.
  • Often SIGURG signal is handled for when the URG flag is set on a TCP packer when using Out-Of-Band transmissions in TCP. Example: Telnet.
  • Flags set by signal handlers should be declared volatile or compiler may optimize them away.
  • Look into the way different signals interact. IE, one handler sets EID=0 before logging and calling exit(1): can we interrupt and interact with the process which is now EID 0?
  • Calling longjmp() when the function that called setjmp() has returned will land it in a stack of unkown data
  • sigprocmask() should be used to prevent signal delivery whilst a signal handler is being run
  • Signals cheatsheet:
    • SIGURG
    • SIGPIPE
    • SIGABRT
    • SIGSEGV

Typos

  • typos: assignment vs comparison, && vs &, == vs =
  • missing curly braces around function/if statement (think goto fail)

String Stuff

Functions

  • snprintf behavior is different unix vs windows for src longer than dst:
    • Windows: return -1, NUL termination not guaranteed
    • Unix: NUL is guaranteed, return length that would’ve been written if dst was sufficiently large
  • strncpy won’t NUL terminate when length of src > length of dst
  • strncat: the size variable should be bytes left not total size or sizeof()
    • and sizeof(buf) - strlen(buf) won’t cut it: it needs to account for NUL
    • and when fixing the above, be careful for int underflow doing sizeof(buf) - strlen(buf) -1 !!
  • strlcpy will always NUL terminate, but return value is size of src not including NUL. Don’t use that value for doing calculations on space remaining in a buffer.
  • strncmp(value, user_controlled, strlen(user_controlled)); will pass for a user_controlled value of ""

Red Flags!

  • Copy/counting/iteration loop bounding
  • Any type of character expansion or escaping
  • Searching beyond a string (ie strchr())
  • Doing searching or decoding where you stop on a char (ie %) and then +=2 past it, not checking for NULL

Wide Strings

  • Danger: 2 bytes on windows, 4 bytes on linux
  • wrong: malloc(strlen(wstr)); wide string can contain Null anywhere
  • wrong: malloc(wcslen(wstr)); count of chars, not underlying size
  • right: malloc(wcslen(wstr) * sizeof(wchar_t)); count chars, multiply by size on arch

String Null Termination

  • copying not enough bytes, leading to the loss of Null terminator (ie strncpy(a,b,sizeof(a));)
    • Quoting docs: “No null-character is implicitly appended at the end of destination if source is longer than num. Thus, in this case, destination shall not be considered a null terminated C string (reading it as such would overflow).”
  • off-by one, leaving not enough space for Null terminator
  • scanf() with a %10c specifier to read 10 chars, will not Null terminate
  • strncpy() will pad with zeros, strncpy_s() will not

Embedded Metacharacters

The following process is used to find these vulnerabilities:

  • Look for code which parses or interprets metacharacters (ie \n or \0 or : or %TEMP%)
  • Check all paths for input flow
  • Examine what filtering occurs and whether it can be bypassed to sneak metacharacters in
  • Analyze exploitability

Example:

Passwords are stored like so: username:password\nadmin:t0psekret\n

If you can set a password of Password1!\nnewUser:pass\n then you’ve injected a new user.

Metacharacter filtering

  • Remember the list of bad chars contextual, and also very large
    • not just what’s on the keyboard - don’t forget escapes like \n or %00
  • The classic tricks of letting the filter algo remove chars, resulting in your attack string

Metacharacter encoding

  • Key point: watch the order of encoding and security decisions. Encoding can often be used to bypass/interfere with security.
    • ie checking for bad chacters, and then doing decoding of hex.

Unicode

  • TODO: go off and read the spec.
  • TODO: write a UTF-8 encoder/decoder to fully understand the spec
  • Unicode 3.0 mandates that only the shortest UTF-8 string is used, but some implementations still allow:
    • / == 0x2f == 0xC0 0xAF .. useful for bypassing filters that are searching for 0x2f
  • UTF-8 can be 1,2,3,4 or 5 bytes
  • UTF-16 can either be a single 16-bit value or two 16-bit values
  • Operations on Unicode can change underlying size requirements: http://unicode.org/reports/tr36/
  • Homographic attacks
  • What happens to a decoding routine when you put a first byte of a multi-byte sequence, folowed by NUL?

Other string stuff

fgets Note: fgets() returns when buf is full or it encounters \n or EOF. This means that NUL characters can be anywhere - this catches developers off-guard. For example, byte 0 can be a NUL, causing calculations based on strlen() to go wild.

Truncation: snprintf(buf, sizeof(buf), "/data/profiles/%s.txt", username);. What’s wrong here? With a long enough username, the file extension can be truncated. And you can use nice fillers if you want a specific file: /./././ or /////.

Also, think of something like SQL injection snprintf(buf, sizeof(buf), "SELECT * FROM x WHERE detail LIKE '%%%s%%' AND account == 'dean'",input); where you can truncate the account == clause.

Directory Traversal: You know what this is. /../../ and friends

  • don’t forget that regexes do not correctly filter this.
>>> print(re.sub(r"\.\.\/", "", "....//....//....//etc/passwd"))
../../../etc/passwd

Other

Crypto

  • RNG
  • cipher choice
  • correct application of MAC
  • cipher mode
  • key mgmt
  • Zeroing key material

sizeof()

The use of sizeof() against an underlying pointer value instead of dereferencing the value first will lead to the wrong size.

For example, a potential memleak of uninitialized memory:

AnObj *o = (AnObj *) malloc(sizeof(AnObj));
memset(o, 0x0, sizeof(o));
  • check for misuse of sizeof: it works on a char[] but for char * you need to deref to get the correct size

General C Nuances

  • order of evaluation is undefined
    • printf("%d %d\n",i++, i++); - no guarantees
    • doThing(something(),something()); - no guarantees

Auditing Variable Use

Variable Relationships
  • The key idea is to look for variables that are related
    • a pointer into a buffer and it’s length variable
    • two related length fields
    • variables representing level of nested-ness
    • anything that should move in lockstep
  • Look for the code paths which allow those assumptions to be broken
  • A good place to start: structs and their _init() functions which describe variable relationships
    • ie length, offset, end variables. Desynchronizing these likely ends to corruption
Variable Initialization
  • Everywhere a variable is read or used
    • Is there a code path via which it is used without initialization?

#TODO revisit the Variable Use section of Chapt 7 TAOSSA

Auditing Control Flow

Loops
  • Does an exit/error condition leave variables in an inconsistent state?
  • Does the loop have a valid exit condition?
  • Has the length/exit condition been calculated correctly (off-by-one, cast errors)
  • posttest vs pretest
Switch
  • forgetting to include a break

Auditing Functions

  • Forgetting to check the return value at all? ex: malloc()
  • Are return values ignored or misinterpreted?
  • Are arguments supplied incorrectly?
  • Are arguments updated in an unexpected way?
  • Do any global variables get modified in an unexpected way (side-effects)
  • How may an API/function be used incorrectly?
    • IE memcpy assumes certain things. same with allocators. wrapping?
  • There may be a helper function which basically wraps memcpy(). Go audit each one of those and treat them as memcpy().
  • check that the type returned by a function is the same as type of variables that take value from that function
    • IE caller expects a bool and -1 is returned as err
  • Where you may not be able to input a value due to checks (say -1), can you get a function to fail and return an error condition that does it for you? IE get get_uint() to fail and return your -1. Extra lolz if the return value is assigned into an unsigned type, error -1 will go large.

Function Side Effects

  • referentially transparent: no side-effects; the function could be replaced with the return value
  • referentially opaque: causes side-effects, ie global variables or pass-by-reference variables.
  • realloc() returns a pointer to a new region of memory. Any old references (say, in callers) may contain outdated references after.

The Function Audit Log

Name read_data

Location file.c:34

Description Reads data from supplied socket and allocates buffer for data

Xrefs process_request.c:324

Prototype int read_data(int sockfd, char **buffer, int *length)

Return Value Type 32-bit signed int

Return value meaning Indicates error: 0 is success, -1 for error

Error Conditions calloc() failure when allocating MAX_SIZE bytes

Arg1 Prototype wchar_t *dest

Arg1 Meaning Destination buffer where data is copied from source buffer

Arg2 Prototype wchar_t *src

Arg2 Meaning Source buffer where wide chars are copied from

Arg3 Prototype size_t len

Arg3 Meaning Maximum size in wide chars of dest buffer (not including a NUL terminator)

Implications

  • NUL termination isn’t guaranteed.

  • len param doesn’t include null terminator character, so null can be written out of bounds if len is exact size of buffer/2

  • the len param is in wide chars; callers may accidentally use sizeof(buf), resulting in an overflow

  • If 0 is supplied as len, it’s decremented to -1 and an infinite copy occurs

Auditing Memory Use

  • Use an ACC Log (Allocation, Check, Copy)
  • Any loops doing copies need to be double, triple checked for terminating conditions. quadruple.
    • is there any possible case under which it may overflow or not terminate the loop correctly?
    • write out a manual log
    • write out a code snippet to verify
  • order of operations matters. Again, write out code or execute/visualise what happens.
  • are length/allocation checks 100% the same as copy loops in all cases?

Allocation & Free problems

  • Whenever these two things get out of sync. In any way. Remember samba CVE-2015-0240: 
    1255 NTSTATUS _netr_ServerPasswordSet(struct pipes_struct *p,
1256                  struct netr_ServerPasswordSet *r)
1257 {
1258     NTSTATUS status = NT_STATUS_OK;
1259     int i;
1260     struct netlogon_creds_CredentialState *creds;
1261
1262     DEBUG(5,("_netr_ServerPasswordSet: %d\n", __LINE__));
1263
1264     become_root();
1265     status = netr_creds_server_step_check(p, p->mem_ctx,
1266                           r->in.computer_name,
1267                           r->in.credential,
1268                           r->out.return_authenticator,
1269                           &creds);
1270     unbecome_root();
1271
1272     if (!NT_STATUS_IS_OK(status)) {
1273         DEBUG(2,("_netr_ServerPasswordSet: netlogon_creds_server_step failed. Rejecting auth "
1274             "request from client %s machine account %s\n",
1275             r->in.computer_name, creds->computer_name));
1276         TALLOC_FREE(creds);
1277         return status;
1278     }

    The creds structure isn’t initialized to NULL, so chase it down the function calls to see if there is any code path in which it never is, prior to the TALLOC_FREE(creds) call. Meet the prereqs to trigger the bug.

  • Code paths that can lead to a double-free?
  • Force errors which lead to allocation functions returning early and not allocating/initializing a value
  • Is there a code path where a pointer or crucial value (or ANY value) is left uninitialized? and then used? Ideally all values should be initialized uint32_t val1 = 0; //in case
  • Any use of a pointer which wasn’t initialized.
  • calloc can wrap when the SIZE and the COUNT multiply to become too big.. this is library-reliant, TODO see if glibc is vuln?

Custom Memory Allocators

  • What happens to an allocation of zero bytes?
  • Does the allocator round up? does it do it safely always, or could it wrap?
    • of particular concern in realloc-like functions which naturally always do arithmetic
  • Does an alloc wrapper take a uint32? on an LP64 system, it will truncate size_t values
  • See allocator scorecard in TAOSSA:378
  • Double frees: are there any code paths at all?
    • Don’t forget: realloc() will free on a size of zero for Unices, but C11 actually says IMPLDEF todo: what does it return? NULL?

Unix Things

Process Privileges

Correct order of dropping privs:

  • setgroups()
  • setgid()
  • setuid()
  • Anything else is likely to be vulnerable.. ie setgroups() doesn’t work when not uid==0

TODO: re-study privilege management.

IDs in a process:

  • Real user ID: user who ran the program
  • Saved set-user-ID: if program is configured as SUID, runs as the user who owns the file
  • Effective user ID: The actual ID used when permission checks run in kernel.

Important thing to note: all the setuid(), setgid() and family behave differently on different operating systems and whether UID 0 or not. In particular, those two functions do not behave correctly unless you are UID 0. This is a mine-field, reread chapter 9 TAOSSA.

Temporary dropping of privileges will prevent an arb file read or similar, but if you have code injection/memory corruption, you can just reinstate saved ID values.

Concurrency/Synchronicity/Race conditions

Terms
  • atomicity: an operation which requires atomicity must be executed sequentially in one go
  • reentrancy: a function that is re-entrant can be interrupted and called again
    • Any function that uses global variables is by definition non-reentrant
vulns
  • non-reentrant functions can be a problem across threads
    • users[curr_idx] = name; curr_idx++; is not reentrant and thus dangerous when used concurrently
Threads
  • Main linux API is PThreads
  • two key types which have a collection of init/use/delete functions
    • mutex
    • condition - threads can wait for a condition. that condition can either be signaled (a single waiting thread will be notified) or broadcast (all waiting threads notified)

greppables

  • memcpy/memmove/memset
  • read/recvfrom
  • bcopy/bzero
  • malloc/calloc/realloc/free
  • mmap
  • scanf/strcpy/strncpy/strcat/strncat/snprintf
  • execve/exec*
  • open/fopen
  • goto
  • getenv
  • setjmp/longjmp
  • [..]printf & syslog
    • format string attacks
  • int size/len
  • #define
  • return -1
  • copy_from_user
  • stat/access
  • sizeof
    • both because of return-val size_t comparisons/casts, and forgetting to deref a ptr
  • srand & rand
  • ntohs, ntohl, htons, htonl
    • Parsing from the network
  • O_CREAT
    • if there’s no O_EXCL then this could open an existing file
  • ../
    • are they doing this correctly?
  • mktemp, tempnam, tmpnam, tmpfile, mkdtemp, mkstemp

C++

  • use of delete instead of delete[] on a non-scalar (ie array) of objects is undefined
  • C++ STL containers contain a lot of common issues: iterators are pointers and can go out of bounds (usually backed by simple data types)
  • free()ing things that should not be free()’d. for example, stuff that’s been created via new
  • type confusion: objects being reinterpreted as another. C++: dynamic_cast reinterpret_cast static_cast. return type deduction/auto can make this easier to make accident
  • VARIANT/NPVARIANT is an MS style of union/struct that can be reinterpreted or type confused easily
  • two unique_ptr to a single object will result in two calls to the destructor when the two references go out of scope
  • shared_ptr can result in double-free if make_shared is not used. IE two separate shared_ptr instances to a single object instead of copying the shared pointer which will do ref counting
  • a shared_ptr or unique_ptr that has it’s .get() method called returns a raw pointer. That bypasses reference counting and likely leads to UAF.
  • anything that can increment/decrement reference counter out of sync with actual object counts could overflow or use-after-free
  • always audit overloaded operators such as operator= because they change expected behaviours

TODOTIDY/Other

  • assigning data into a struct direct from network: there could be padding that screws this up
  • Time-Of-Check/Time-Of-Use vulnerabilities over privilege boundaries; double-fetch vulns against a kernel in particular
  • are there process flows where a return value fails open? ideally you want to see a func first line of uint32_t result = FAILURE; //prevent fail open
  • check for resources that only allow a single handle; can they be locked out? is there a code path where close_X is not called after an open_X ?

Compiler Tricks

  • gcc -E source.c will run the preprocessor over source file[s]
  • gcc -funsigned-char force the char datatype to be unsigned instead of standard signed
  • gcc -S -masm=intel thing.c will output assembly, in intel flavor
  • TODO all the ASan stuff

Resources

  • Chris Rohlfs MMS training
  • TAOSSA
  • Secure C/C++ RCS
  • Bughunters Diary
  • CVEs