Spot the Vuln

Finding exploits in source code for various programming languages!

[C] Check Again

Initial Thoughts

The size of the header structure is most likely 8, so the first if statement check if length > 1024+8-1 or length > 1031. I see that the buffer is of size 1024. During the second read_sock function call, it calls to buffer with length-8, this may lead to an overflow issue since only the upper bound of length was checked.

Further Analysis

The function get_data in theory takes in a socket and parses data into a buffer. The key vulnerability is on line 19:

if(read_sock(sock, buffer, length-sizeof(struct header)) <= 0) {

If the length variable is smaller than the header size, this will cause an overflow. For example, assume that the first message read the string "hello". The first read_sock would have read the message of length 5 and stored a header structure with length 5. This would have passed the comparison because length = 5 < 1031 but when it get's to the second read_sock function, it would call read_sock(sock, buffer, 5-8=-3). The -3 would wrap around to INT_MAX and allow the user to write much more than 1024 characters to the buffer.

[Rust] Beyond the Grave

Initial Thoughts

This is a piece of a rust code. It seems like it's declaring a public function called sign which creates a variable p that gets assigned two values, (1) being BioSlice data which is a pointer, and (2) which is a NULL pointer. There's a line that starts with unsafe, where it sets a variable named cms to cvt_p(CMS_sign(p)) which I'm guessing is a signature of the previous set data.

Further Analysis

This is a rust specific issue that has to do with rust object lifetimes. Within each curly brace set, a lifetime exists. When it goes out of scope, any variable defined within that lifetime gets freed. The first function actually sets the variable p to data if there is a return, else None if the pointer in NULL. The problem happens in the following line of code.

let cms = cvt_p(CMS_sign(p));

Here, p is going to get the value of the BioSlice that is newly created in that match statement. That data will get freed after leaving scope. The unsafe procedure let's you still access the p data without throwing an error from the compiler.

[Python] Regex

def get(url):
    allow = r"^https://api.safe.com/"
    if not re.match(allow, url):
        raise ValueError("Invalid URL")
    
    res = requests.get(url, timeout=5)
    res.raise_for_status()
    return res

Initial Thoughts

The method get is very simple and basically uses regular expressions to search for a website. If the search parameter url matches the allowed link, it will request the webpage otherwise raise a value error. The only location for this to have a vulnerability is in the regex on line 2.

Further Analysis

The regex is as follows:

allow = r"^https://api.safe.com/"

In regular expressions, special characters often have a meaning, and do not reflect the pure ASCII value. For example, here are some of the values:

  • . (Dot): Matches any single character except a newline (\n) by default

  • * (Asterisk): Matches zero or more occurrences of the preceding element

  • + (Plus): Matches one or more occurrences of the preceeding element

  • ? (Question): Matches zero or one occurrence of the preceeding element

  • ^ (Caret): Matches the start of the string

  • $ (Dollar): Matches the end of the string

  • \ (Backslash): Escapes spcial characters, allowing for literal matching

So the earlier would allows for the . dots to act as wildcards allowing for the url to match any of the non true values:

  • https://api-safe.com/

  • https://api_safe.com/

  • https://api<SOME_WILDCARD_VALUE>safe.com/

The second . dot is also a wild card, but by not including one, it would no longer be a valid website to fetch. Although this regex is very good at preventing other problems the best way to make it safe would be to making the . dots literal:

allow = r"^https://api\.safe\.com/"

[C] Just be Positive

#define MAX_PACKET 1024
char *read_data(int sockfd) {
    char buf[MAX_PACKET];
    int length = packet_get_init(sockfd);
    
    if(length < 0) {
        length = length * -1;
    }
    
    if(length >= MAX_PACKET) {
        return NULL;
    }
    
    if(read(sockfd, buf, length) <= 0) {
        return NULL;
    }
    
    /* Continue processing */
}

Initial Thoughts

The read_data function returns a pointer to a string of chars and takes parameter sockfd referring likely to an established channel socket. Buffer is of size 1024 and length is expected to be some positive length of the data that is to be received on sockfd. The program reads an integer into the variable length and then does a check to see if the value is negative. If it is, it changes it by multiplying the length value by "-1".

The program then checks if length is more than the MAX_PACKET length, and lastly reads the data of size length into the buffer.

Further Analysis

This required some extra reading to understand properly. The value INT_MIN which is the most negative signed number you could make with an integer value and an inherit definition in C, does not work as expected when used in the first bounds check modification.

length = length * -1; // length = INT_MIN * -1

This is because of a side-effect of how signed integers work leading the following to be true: INT_MIN * -1 == INT_MIN.

The ending result, is that the first bounds check is bypassed since INT_MIN is still a negative value after the equation executes, and when read is eventually called, the length is converted to an unsigned int resulting in a far too large write into buf causing a stack-based overflow.

The stem of this problem was trying to correct the user input rather than rejecting it. Although mathematically sound, the above specified code doesn't hold true in a digital sense for most CPUs because of something called "Two's Complement" which is how the CPU stores values that can be negative or positive.

Last updated