Copilot commented on code in PR #12908:
URL: https://github.com/apache/trafficserver/pull/12908#discussion_r2879328846
##########
src/tscore/ink_cap.cc:
##########
@@ -463,6 +463,11 @@ ElevateAccess::acquirePrivilege(unsigned priv_mask)
++cap_count;
}
+ if (priv_mask & ElevateAccess::CHOWN_PRIVILEGE) {
+ cap_list[cap_count] = CAP_CHOWN;
+ ++cap_count;
+ }
+
ink_release_assert(cap_count <= sizeof(cap_list));
Review Comment:
The bounds-check assertion at line 471 compares `cap_count` (the number of
capabilities, max 4) against `sizeof(cap_list)` (the byte size of the array,
which is `4 * sizeof(cap_value_t)` = 16 bytes on typical platforms). This means
the assertion is effectively `cap_count <= 16`, which will never catch an
actual out-of-bounds write. It should use the element count instead: `cap_count
<= sizeof(cap_list) / sizeof(cap_list[0])`.
This PR brings `cap_count` up to a potential maximum of 4, exactly matching
the array dimension, so the array is not overflowed now — but the guard is
broken. Any future capability addition will silently overflow `cap_list`
without the assertion firing.
```suggestion
ink_release_assert(cap_count <= sizeof(cap_list) / sizeof(cap_list[0]));
```
##########
include/tscore/ink_cap.h:
##########
@@ -81,8 +81,9 @@ class ElevateAccess
FILE_PRIVILEGE = 0x1u, ///< Access filesystem objects with privilege
TRACE_PRIVILEGE = 0x2u, ///< Trace other processes with privilege
LOW_PORT_PRIVILEGE = 0x4u, ///< Bind to privilege ports.
- OWNER_PRIVILEGE = 0x8u ///< Bypass permission checks on operations
that normally require
+ OWNER_PRIVILEGE = 0x8u, ///< Bypass permission checks on operations
that normally require
/// filesystem UID & process UID to match
+ CHOWN_PRIVILEGE = 0x10u ///< Change file ownership
Review Comment:
The `CHOWN_PRIVILEGE` entry is not aligned to the same column as the other
enum values. All previous entries use column-aligned spacing before `=` to keep
the values visually aligned (e.g., `FILE_PRIVILEGE = 0x1u`,
`OWNER_PRIVILEGE = 0x8u`), but `CHOWN_PRIVILEGE = 0x10u` has only a single
space before `=`, breaking the alignment pattern.
```suggestion
CHOWN_PRIVILEGE = 0x10u ///< Change file ownership
```
--
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]
