bneradt commented on code in PR #13356:
URL: https://github.com/apache/trafficserver/pull/13356#discussion_r3574914065


##########
include/tscore/ink_inet.h:
##########
@@ -1610,9 +1611,9 @@ struct UnAddr {
 
   UnAddr() { _path[0] = 0; }
 
-  UnAddr(self const &addr) { strncpy(_path, addr._path, TS_UNIX_SIZE); }
-  explicit UnAddr(const char *path) { strncpy(_path, path, TS_UNIX_SIZE - 1); }
-  explicit UnAddr(const std::string &path) { strncpy(_path, path.c_str(), 
TS_UNIX_SIZE); }
+  UnAddr(self const &addr) { ink_strlcpy(_path, addr._path, TS_UNIX_SIZE); }
+  explicit UnAddr(const char *path) { ink_strlcpy(_path, path, TS_UNIX_SIZE); }
+  explicit UnAddr(const std::string &path) { ink_strlcpy(_path, path.c_str(), 
TS_UNIX_SIZE); }
 
   explicit UnAddr(sockaddr const *addr) { this->assign(addr); }

Review Comment:
   This calls assign() without initializing _path. When passed a runtime null 
pointer, 
[assign()](https://github.com/apache/trafficserver/blob/86dc0348eb0a7cfabb6e6298c676632f197f8875/include/tscore/ink_inet.h#L1652-L1659)
 leaves the buffer untouched. Copying that object now calls ink_strlcpy, which 
scans the uninitialized buffer for a terminator and may read beyond it. 
Initialize _path for every constructor—such as char _path[TS_UNIX_SIZE]{}—or 
delegate the pointer constructor to UnAddr() and add a null-address test. 
Similar issues apply to the UnAddr constructors below this for passed null 
values. The one at 1623 seems to have a nullptr dereference even in these cases.



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

Reply via email to