From 92bfc1c00431f28dfaf15968f41e62181580e08e Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Wed, 16 Jan 2019 17:06:41 +0100 Subject: Use OString for memory management of osl_psz_createPipe ...and make sure that the generated name is actually short enough to be stored in a sockaddr_un::sun_path. (oslPipeImpl::m_Name is only used to store such a name, to be used in osl_closePipe, so just make it of the same size to guarantee that copying between the two with strcpy will always work.) Change-Id: I6d1d0c5518e6d09eff129d682a1a334d12201e90 Reviewed-on: https://gerrit.libreoffice.org/66469 Tested-by: Jenkins Reviewed-by: Stephan Bergmann --- sal/osl/unx/pipe.cxx | 67 ++++++++++++++++++---------------------------------- 1 file changed, 23 insertions(+), 44 deletions(-) (limited to 'sal/osl/unx/pipe.cxx') diff --git a/sal/osl/unx/pipe.cxx b/sal/osl/unx/pipe.cxx index c07d1887bc86..a4f1594a1896 100644 --- a/sal/osl/unx/pipe.cxx +++ b/sal/osl/unx/pipe.cxx @@ -38,9 +38,6 @@ #define PIPEDEFAULTPATH "/tmp" #define PIPEALTERNATEPATH "/var/tmp" -#define PIPENAMEMASK "OSL_PIPE_%s" -#define SECPIPENAMEMASK "OSL_PIPE_%s_%s" - static oslPipe osl_psz_createPipe(const sal_Char *pszPipeName, oslPipeOptions Options, oslSecurity Security); static struct @@ -147,53 +144,35 @@ static oslPipe osl_psz_createPipe(const sal_Char *pszPipeName, oslPipeOptions Op size_t len; struct sockaddr_un addr; - sal_Char name[PATH_MAX+1]; - size_t nNameLength = 0; - bool bNameTooLong = false; + OString name; oslPipe pPipe; if (access(PIPEDEFAULTPATH, W_OK) == 0) - strncpy(name, PIPEDEFAULTPATH, sizeof(name)); + name = PIPEDEFAULTPATH; else if (access(PIPEALTERNATEPATH, W_OK) == 0) - strncpy(name, PIPEALTERNATEPATH, sizeof(name)); + name = PIPEALTERNATEPATH; else { - auto const path = getBootstrapSocketPath (); - if (path.isEmpty() || sal_uInt32(path.getLength()) > sizeof(name) - 1) { - return nullptr; - } - std::memcpy(name, path.getStr(), sal_uInt32(path.getLength()) + 1); + name = getBootstrapSocketPath (); } - name[sizeof(name)-1] = '\0'; // ensure the string is NULL-terminated - nNameLength = strlen(name); - bNameTooLong = nNameLength > sizeof(name) - 2; + name += "/"; - if (!bNameTooLong) + if (Security) { - size_t nRealLength = 0; - - strcat(name, "/"); - ++nNameLength; - - if (Security) - { - sal_Char Ident[256]; + sal_Char Ident[256]; - Ident[0] = '\0'; + Ident[0] = '\0'; - OSL_VERIFY(osl_psz_getUserIdent(Security, Ident, sizeof(Ident))); + OSL_VERIFY(osl_psz_getUserIdent(Security, Ident, sizeof(Ident))); - nRealLength = snprintf(&name[nNameLength], sizeof(name) - nNameLength, SECPIPENAMEMASK, Ident, pszPipeName); - } - else - { - nRealLength = snprintf(&name[nNameLength], sizeof(name) - nNameLength, PIPENAMEMASK, pszPipeName); - } - - bNameTooLong = nRealLength > sizeof(name) - nNameLength - 1; + name += OStringLiteral("OSL_PIPE_") + Ident + "_" + pszPipeName; + } + else + { + name += OStringLiteral("OSL_PIPE_") + pszPipeName; } - if (bNameTooLong) + if (sal_uInt32(name.getLength()) >= sizeof addr.sun_path) { SAL_WARN("sal.osl.pipe", "osl_createPipe: pipe name too long"); return nullptr; @@ -229,7 +208,7 @@ static oslPipe osl_psz_createPipe(const sal_Char *pszPipeName, oslPipeOptions Op SAL_INFO("sal.osl.pipe", "new pipe on fd " << pPipe->m_Socket << " '" << name << "'"); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, name, sizeof(addr.sun_path) - 1); + strcpy(addr.sun_path, name.getStr()); // safe, see check above #if defined(FREEBSD) len = SUN_LEN(&addr); #else @@ -241,7 +220,7 @@ static oslPipe osl_psz_createPipe(const sal_Char *pszPipeName, oslPipeOptions Op struct stat status; /* check if there exists an orphan filesystem entry */ - if ((stat(name, &status) == 0) && + if ((stat(name.getStr(), &status) == 0) && (S_ISSOCK(status.st_mode) || S_ISFIFO(status.st_mode))) { if (connect(pPipe->m_Socket, reinterpret_cast< sockaddr* >(&addr), len) >= 0) @@ -251,7 +230,7 @@ static oslPipe osl_psz_createPipe(const sal_Char *pszPipeName, oslPipeOptions Op return nullptr; } - unlink(name); + unlink(name.getStr()); } /* ok, fs clean */ @@ -267,9 +246,9 @@ static oslPipe osl_psz_createPipe(const sal_Char *pszPipeName, oslPipeOptions Op depends on umask */ if (!Security) - chmod(name,S_IRWXU | S_IRWXG |S_IRWXO); + chmod(name.getStr(),S_IRWXU | S_IRWXG |S_IRWXO); - strncpy(pPipe->m_Name, name, sizeof(pPipe->m_Name) - 1); + strcpy(pPipe->m_Name, name.getStr()); // safe, see check above if (listen(pPipe->m_Socket, 5) < 0) { @@ -279,7 +258,7 @@ static oslPipe osl_psz_createPipe(const sal_Char *pszPipeName, oslPipeOptions Op // unrelated, as it would fail if name existed at that point in // time: // coverity[toctou] - this is bogus - unlink(name); /* remove filesystem entry */ + unlink(name.getStr()); /* remove filesystem entry */ close(pPipe->m_Socket); destroyPipeImpl(pPipe); return nullptr; @@ -289,7 +268,7 @@ static oslPipe osl_psz_createPipe(const sal_Char *pszPipeName, oslPipeOptions Op } /* osl_pipe_OPEN */ - if (access(name, F_OK) != -1) + if (access(name.getStr(), F_OK) != -1) { if (connect(pPipe->m_Socket, reinterpret_cast< sockaddr* >(&addr), len) >= 0) return pPipe; @@ -357,7 +336,7 @@ void SAL_CALL osl_closePipe(oslPipe pPipe) SAL_INFO("sal.osl.pipe", "osl_destroyPipe : Pipe Name '" << pPipe->m_Name << "'"); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, pPipe->m_Name, sizeof(addr.sun_path) - 1); + strcpy(addr.sun_path, pPipe->m_Name); // safe, as both are same size nRet = connect(fd, reinterpret_cast< sockaddr* >(&addr), sizeof(addr)); if (nRet < 0) -- cgit