[BUG] Multiple Intra-Object Overflows in ParentSelection.cc (hash_string)

[neosys007]

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 When receiving a SIGTERM wait() for children #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:

4. Memory Corruption: memcpy will overflow the hash_string buffer in the pRecord structure.

5. 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.

6. 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.

7. 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