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