neosys007 opened a new issue, #12973:
URL: https://github.com/apache/trafficserver/issues/12973
I am reporting two related memory safety vulnerabilities (intra-object
overflows) in src/proxy/ParentSelection.cc. These issues involve an
inconsistent length validation
when parsing parent configuration records, leading to potential buffer
overflows in the hash_string field.
1. Vulnerability Analysis
In ParentSelection.cc, the code parses parent server strings from the
configuration. It correctly validates the length of the hostname field but
completely fails to
validate the length of the hash_string field before performing a memcpy.
Data Structure (ParentSelection.h):
1 struct pRecord : ATSConsistentHashNode {
2 char hostname[MAXDNAME + 1]; // 1025 bytes
3 ...
4 char hash_string[MAXDNAME + 1]; // 1025 bytes (Target of overflow)
5 };
The Logic Flaw:
The code includes a guard for hostname (line 552):
if (tmp - current > MAXDNAME) { ... errPtr = "Parent hostname is too
long"; ... }
However, later in the same function, it performs a memcpy into hash_string
using strlen(tmp3) without any bounds check.
2. Vulnerable Locations
Location #1: Primary Parents (src/proxy/ParentSelection.cc:572)
1 if (tmp3) {
2 // strlen(tmp3) is UNCHECKED and can exceed MAXDNAME
3 memcpy(this->parents[i].hash_string, tmp3 + 1, strlen(tmp3));
4 this->parents[i].name = this->parents[i].hash_string;
5 }
Location #2: Secondary Parents (src/proxy/ParentSelection.cc:587)
1 if (tmp3) {
2 // Symmetrical flaw for secondary_parents array
3 memcpy(this->secondary_parents[i].hash_string, tmp3 + 1,
strlen(tmp3));
4 this->secondary_parents[i].name =
this->secondary_parents[i].hash_string;
5 }
3. Impact
An administrator or an attacker capable of influencing the parent.config
(or other parent selection configuration) can provide a string that exceeds
MAXDNAME (1024 bytes).
This results in:
1. Memory Corruption: memcpy will overflow the hash_string buffer in the
pRecord structure.
2. Heap Overwrite: Since parents is an array of pRecord objects, an
overflow in parents[i] will overwrite the metadata or member data of
parents[i+1] or adjacent heap
allocations.
3. Instability: This corruption can lead to inconsistent
consistent-hashing results, crashes (DoS), or potentially arbitrary code
execution if critical pointers within
the ATSConsistentHashNode are corrupted.
4. Suggested Fix
Apply a length check to tmp3 similar to the one used for the hostname
before performing the memcpy:
1 if (tmp3) {
2 size_t hash_len = strlen(tmp3);
3 if (hash_len > MAXDNAME) {
4 // Handle error: hash string too long
5 }
6 memcpy(this->parents[i].hash_string, tmp3 + 1, hash_len);
7 }
Best regards,
Pengpeng Hou
ISCAS
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
