Invalid handling of unix paths with maximum length

[degasus]

The definition of the struct

struct sockaddr_un
  {
    __SOCKADDR_COMMON (sun_);
    char sun_path[108];		/* Path name.  */
  };

Allows a path with up to 108 characters. If all chars are used, there is no null termination. So all accessors of this path should check for the size:

--- a/sockpp/include/sockpp/unix_address.h
+++ b/sockpp/include/sockpp/unix_address.h
@@ -125,7 +125,7 @@ public:
 	 * Gets the path to which this address refers.
 	 * @return The path to which this address refers.
 	 */
-	std::string path() const { return std::string(addr_.sun_path); }
+	std::string path() const { return std::string(addr_.sun_path, strnlen(addr_.sun_path, MAX_PATH_NAME)); }
 	/**
 	 * Gets the size of the address structure.
 	 * Note: In this implementation, this should return sizeof(this) but
@@ -170,7 +170,7 @@ public:
 	 *  	   "unix:<path>"
 	 */
 	std::string to_string() const {
-		return std::string("unix:") + std::string(addr_.sun_path);
+		return std::string("unix:") + path();
 	}
 };
 
--- a/sockpp/tests/unit/test_unix_address.cpp
+++ b/sockpp/tests/unit/test_unix_address.cpp
@@ -65,8 +65,8 @@ TEST_CASE("unix_address path constructor", "[address]") {
 
     // Check the low-level struct
     REQUIRE(AF_UNIX == addr.sockaddr_un_ptr()->sun_family);
-    REQUIRE(0 == strcmp(PATH.c_str(),
-                        (const char*) &addr.sockaddr_un_ptr()->sun_path));
+    REQUIRE(0 == strncmp(PATH.c_str(),
+                        (const char*) &addr.sockaddr_un_ptr()->sun_path, MAX_PATH_NAME));
 
     SECTION("copy constructor") {
         unix_address addr2(addr);
@@ -77,8 +77,8 @@ TEST_CASE("unix_address path constructor", "[address]") {
 
         // Check the low-level struct
         REQUIRE(AF_UNIX == addr2.sockaddr_un_ptr()->sun_family);
-        REQUIRE(0 == strcmp(PATH.c_str(),
-                            (const char*) &addr2.sockaddr_un_ptr()->sun_path));
+        REQUIRE(0 == strncmp(PATH.c_str(),
+                            (const char*) &addr2.sockaddr_un_ptr()->sun_path, MAX_PATH_NAME));
     }
 
     SECTION("sockaddr conversions") {
@@ -91,15 +91,15 @@ TEST_CASE("unix_address path constructor", "[address]") {
 
         // Check the low-level struct
         REQUIRE(AF_UNIX == addr2.sockaddr_un_ptr()->sun_family);
-        REQUIRE(0 == strcmp(PATH.c_str(),
-                            (const char*) &(addr2.sockaddr_un_ptr()->sun_path)));
+        REQUIRE(0 == strncmp(PATH.c_str(),
+                            (const char*) &(addr2.sockaddr_un_ptr()->sun_path, MAX_PATH_NAME)));
     }
 }
 
 TEST_CASE("unix_address sockaddr_un constructor", "[address]") {
     sockaddr_un unaddr;
     unaddr.sun_family = AF_UNIX;
-    strcpy(unaddr.sun_path, PATH.c_str());
+    strncpy(unaddr.sun_path, PATH.c_str(), MAX_PATH_NAME);
 
     unix_address addr(unaddr);
 
@@ -109,8 +109,8 @@ TEST_CASE("unix_address sockaddr_un constructor", "[address]") {
 
     // Check the low-level struct
     REQUIRE(AF_UNIX == addr.sockaddr_un_ptr()->sun_family);
-    REQUIRE(0 == strcmp(PATH.c_str(),
-                        (const char*) &addr.sockaddr_un_ptr()->sun_path));
+    REQUIRE(0 == strncmp(PATH.c_str(),
+                        (const char*) &addr.sockaddr_un_ptr()->sun_path, MAX_PATH_NAME));
 
 	// TODO: Restore this when all address checks in place
 	/*

[fpagliughi]

Yeah, I see what you're saying. But I wouldn't worry about updating the unit tests, other than to perhaps set a better example.

The bigger problem is that I put this off to figure out what to do about the different sizes across different platforms, and never got back to it. On Linux the size is 108, but on other platforms the buffer size is different: 104, 110, 92, etc. I didn't want to limit it to the smallest size from an obscure platform, but wanted to figure out something more portable.

[degasus]

Indeed, I agree about the tests....

What do you think about using sizeof(sockaddr_un::sun_path) ?

[fpagliughi]

Well, took a few years, but the update is in the develop branch!

[degasus]

Thanks for the fix and for the update. Honestly, I totally forgot about this, but I'm glad that it made it in :-)

[fpagliughi]

In Release v0.8.1