Re: [PR] slice: fix stpcpy off-by-one for header value extraction [trafficserver]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #13181:
URL: https://github.com/apache/trafficserver/pull/13181#discussion_r3337468714


##########
plugins/slice/HttpHeader.cc:
##########
@@ -168,33 +168,35 @@ HttpHeader::removeKey(char const *const keystr, int const keylen)
 bool
 HttpHeader::valueForKey(char const *const keystr, int const keylen, char *const valstr, int *const vallen, int const index) const
 {
-  if (!isValid()) {
+  if (nullptr == valstr || nullptr == vallen) {
+    return false;
+  }
+  if (!isValid() || index < -1) {
     *vallen = 0;
     return false;
   }
 
   bool status = false;
 
+  int const valcap = *vallen;
+  *vallen          = 0;
+  valstr[0]        = 0;
+

Review Comment:
   `valueForKey()` unconditionally writes `valstr[0] = 0` after reading the caller-provided capacity from `*vallen`. If the caller passes a capacity of 0 (or a negative value), this becomes an out-of-bounds write even though the function will ultimately return `false`.
   
   Please guard the write (and the rest of the copy logic) with a `valcap > 0` check, returning `false` early for zero/negative capacities.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org