mirror of https://github.com/wolfSSL/wolfssh.git
Reject SCP receive through pre-existing symlinks
parent
00e85522aa
commit
0256f4c8fb
32
src/port.c
32
src/port.c
|
|
@ -689,6 +689,38 @@ int wChmod(const char *path, int mode)
|
|||
}
|
||||
#endif
|
||||
#endif /* NO_FILESYSTEM */
|
||||
|
||||
#if defined(WOLFSSH_HAVE_SYMLINK) && \
|
||||
(defined(WOLFSSH_SFTP) || defined(WOLFSSH_SCP))
|
||||
/* Returns 1 if path is a symbolic link (POSIX) or a reparse point such as a
|
||||
* symlink or junction (Windows), otherwise 0. A non-existent path (stat
|
||||
* fails) is reported as not-a-link so that create requests for a new leaf are
|
||||
* still permitted by the caller. Shared by the SFTP and SCP path-confinement
|
||||
* checks. */
|
||||
int wIsSymlink(const char* path)
|
||||
{
|
||||
int isLink = 0;
|
||||
#ifdef USE_WINDOWS_API
|
||||
WIN32_FILE_ATTRIBUTE_DATA attrs;
|
||||
|
||||
/* Route through WS_GetFileAttributesExA so the wide-char API and any path
|
||||
* trimming match the other Windows file ops. GetFileAttributesEx reports
|
||||
* the link's own attributes; it does not follow the reparse point. */
|
||||
if (path != NULL && WS_GetFileAttributesExA(path, &attrs, NULL) != 0 &&
|
||||
(attrs.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) {
|
||||
isLink = 1;
|
||||
}
|
||||
#else
|
||||
WSTAT_T lst;
|
||||
|
||||
if (path != NULL && WLSTAT(NULL, path, &lst) == 0 && S_ISLNK(lst.st_mode)) {
|
||||
isLink = 1;
|
||||
}
|
||||
#endif
|
||||
return isLink;
|
||||
}
|
||||
#endif /* WOLFSSH_HAVE_SYMLINK && (WOLFSSH_SFTP || WOLFSSH_SCP) */
|
||||
|
||||
#ifndef WSTRING_USER
|
||||
|
||||
char* wstrdup(const char* s1, void* heap, int type)
|
||||
|
|
|
|||
|
|
@ -2220,6 +2220,17 @@ int wsScpRecvCallback(WOLFSSH* ssh, int state, const char* basePath,
|
|||
wolfSSH_CleanPath(ssh, abslut, WOLFSSH_MAX_FILENAME);
|
||||
if (WFOPEN(ssh->fs, &fp, abslut, "wb") != 0) {
|
||||
#else
|
||||
#ifdef WOLFSSH_HAVE_SYMLINK
|
||||
/* refuse to write through a pre-existing symlink, which would
|
||||
* escape the destination directory */
|
||||
if (wIsSymlink(fileName)) {
|
||||
WLOG(WS_LOG_ERROR,
|
||||
"scp: refusing to write through symlink, abort");
|
||||
wolfSSH_SetScpErrorMsg(ssh, "symlink target rejected");
|
||||
ret = WS_SCP_ABORT;
|
||||
break;
|
||||
}
|
||||
#endif
|
||||
if (WFOPEN(ssh->fs, &fp, fileName, "wb") != 0) {
|
||||
#endif
|
||||
WLOG(WS_LOG_ERROR,
|
||||
|
|
@ -2334,6 +2345,17 @@ int wsScpRecvCallback(WOLFSSH* ssh, int state, const char* basePath,
|
|||
wolfSSH_CleanPath(ssh, (char*)basePath, WOLFSSH_MAX_FILENAME);
|
||||
ssh->scpDirDepth++;
|
||||
#else
|
||||
#ifdef WOLFSSH_HAVE_SYMLINK
|
||||
/* WMKDIR returning EEXIST above may have matched a pre-existing
|
||||
* symlink; refuse to follow it out of the destination dir */
|
||||
if (wIsSymlink(fileName)) {
|
||||
WLOG(WS_LOG_ERROR,
|
||||
"scp: refusing to enter symlinked directory, abort");
|
||||
wolfSSH_SetScpErrorMsg(ssh, "symlink in destination path");
|
||||
ret = WS_SCP_ABORT;
|
||||
break;
|
||||
}
|
||||
#endif
|
||||
if (WCHDIR(ssh->fs, fileName) != 0) {
|
||||
WLOG(WS_LOG_ERROR,
|
||||
"scp: unable to cd into directory, abort");
|
||||
|
|
|
|||
|
|
@ -1898,39 +1898,6 @@ int wolfSSH_SFTP_CreateStatus(WOLFSSH* ssh, word32 status, word32 reqId,
|
|||
}
|
||||
|
||||
|
||||
#ifdef WOLFSSH_HAVE_SYMLINK
|
||||
/* Returns 1 if path is a symbolic link (POSIX) or a reparse point such as a
|
||||
* symlink or junction (Windows), otherwise 0. A non-existent path (stat
|
||||
* fails) is reported as not-a-link so that create requests for a new leaf are
|
||||
* still permitted by the caller. */
|
||||
static int SFTP_IsSymlink(const char* path)
|
||||
{
|
||||
int isLink = 0;
|
||||
#ifdef USE_WINDOWS_API
|
||||
WIN32_FILE_ATTRIBUTE_DATA attrs;
|
||||
|
||||
/* GetAndCleanPath produces an SFTP-canonical "/C:/..." path. Route it
|
||||
* through WS_GetFileAttributesExA, which trims the leading slash and uses
|
||||
* the wide-char API like every other Windows file op here; calling
|
||||
* GetFileAttributesA on the raw "/C:/..." form would always fail and
|
||||
* silently disable link detection. GetFileAttributesEx reports the link's
|
||||
* own attributes (it does not follow the reparse point). */
|
||||
if (WS_GetFileAttributesExA(path, &attrs, NULL) != 0 &&
|
||||
(attrs.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) {
|
||||
isLink = 1;
|
||||
}
|
||||
#else
|
||||
WSTAT_T lst;
|
||||
|
||||
if (WLSTAT(NULL, path, &lst) == 0 && S_ISLNK(lst.st_mode)) {
|
||||
isLink = 1;
|
||||
}
|
||||
#endif
|
||||
return isLink;
|
||||
}
|
||||
#endif /* WOLFSSH_HAVE_SYMLINK */
|
||||
|
||||
|
||||
/*
|
||||
* This is a wrapper around the function wolfSSH_RealPath. Since it modifies
|
||||
* the source path value, copy the path from the data stream into a local
|
||||
|
|
@ -2014,7 +1981,7 @@ static int GetAndCleanPath(const char* defaultPath,
|
|||
}
|
||||
saved = s[i];
|
||||
s[i] = '\0';
|
||||
if (SFTP_IsSymlink(s)) {
|
||||
if (wIsSymlink(s)) {
|
||||
ret = WS_PERMISSIONS;
|
||||
}
|
||||
s[i] = saved;
|
||||
|
|
|
|||
173
tests/unit.c
173
tests/unit.c
|
|
@ -4133,6 +4133,174 @@ cleanup:
|
|||
return result;
|
||||
}
|
||||
|
||||
/* A pre-existing symlink in the destination directory must not be followed
|
||||
* out of that directory, neither when entering it as a directory nor when
|
||||
* opening it as a file. */
|
||||
static int test_ScpRecvCallback_SymlinkGuard(void)
|
||||
{
|
||||
#ifndef WOLFSSH_HAVE_SYMLINK
|
||||
/* symlink rejection is compiled out on this configuration */
|
||||
return 0;
|
||||
#else
|
||||
char tmpDir[] = "/tmp/wolfssh_scpXXXXXX";
|
||||
char basePathRaw[PATH_MAX];
|
||||
char outsidePath[PATH_MAX];
|
||||
char linkDirPath[PATH_MAX];
|
||||
char linkFilePath[PATH_MAX];
|
||||
char leakedPath[PATH_MAX];
|
||||
char origCwd[PATH_MAX];
|
||||
char* basePath = NULL;
|
||||
struct stat st;
|
||||
WOLFSSH_CTX* ctx = NULL;
|
||||
WOLFSSH* ssh = NULL;
|
||||
int baseMkdirDone = 0;
|
||||
int outsideMkdirDone = 0;
|
||||
int linkDirDone = 0;
|
||||
int linkFileDone = 0;
|
||||
int origCwdSaved = 0;
|
||||
int ret;
|
||||
int result = 0;
|
||||
|
||||
basePathRaw[0] = '\0';
|
||||
outsidePath[0] = '\0';
|
||||
linkDirPath[0] = '\0';
|
||||
linkFilePath[0] = '\0';
|
||||
leakedPath[0] = '\0';
|
||||
|
||||
if (getcwd(origCwd, sizeof(origCwd)) == NULL)
|
||||
return -840;
|
||||
origCwdSaved = 1;
|
||||
|
||||
if (mkdtemp(tmpDir) == NULL)
|
||||
return -841;
|
||||
|
||||
ret = snprintf(basePathRaw, sizeof(basePathRaw), "%s/scp_target", tmpDir);
|
||||
if (!scpTestSnprintfOk(ret, sizeof(basePathRaw))) {
|
||||
result = -842;
|
||||
goto cleanup;
|
||||
}
|
||||
if (mkdir(basePathRaw, 0755) != 0) {
|
||||
result = -843;
|
||||
goto cleanup;
|
||||
}
|
||||
baseMkdirDone = 1;
|
||||
|
||||
ret = snprintf(outsidePath, sizeof(outsidePath), "%s/outside", tmpDir);
|
||||
if (!scpTestSnprintfOk(ret, sizeof(outsidePath))) {
|
||||
result = -844;
|
||||
goto cleanup;
|
||||
}
|
||||
if (mkdir(outsidePath, 0755) != 0) {
|
||||
result = -845;
|
||||
goto cleanup;
|
||||
}
|
||||
outsideMkdirDone = 1;
|
||||
|
||||
basePath = realpath(basePathRaw, NULL);
|
||||
if (basePath == NULL) {
|
||||
result = -846;
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
ret = snprintf(linkDirPath, sizeof(linkDirPath), "%s/linkdir", basePath);
|
||||
if (!scpTestSnprintfOk(ret, sizeof(linkDirPath))) {
|
||||
result = -847;
|
||||
goto cleanup;
|
||||
}
|
||||
ret = snprintf(linkFilePath, sizeof(linkFilePath), "%s/linkfile", basePath);
|
||||
if (!scpTestSnprintfOk(ret, sizeof(linkFilePath))) {
|
||||
result = -858;
|
||||
goto cleanup;
|
||||
}
|
||||
ret = snprintf(leakedPath, sizeof(leakedPath), "%s/leaked.txt", outsidePath);
|
||||
if (!scpTestSnprintfOk(ret, sizeof(leakedPath))) {
|
||||
result = -859;
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
|
||||
if (ctx == NULL) {
|
||||
result = -848;
|
||||
goto cleanup;
|
||||
}
|
||||
ssh = wolfSSH_new(ctx);
|
||||
if (ssh == NULL) {
|
||||
result = -849;
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
/* NEW_REQUEST changes the working directory into basePath */
|
||||
ret = wsScpRecvCallback(ssh, WOLFSSH_SCP_NEW_REQUEST, basePath,
|
||||
NULL, 0, 0, 0, 0, NULL, 0, 0, NULL);
|
||||
if (ret != WS_SCP_CONTINUE) {
|
||||
result = -850;
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
/* plant a directory symlink pointing outside basePath */
|
||||
if (symlink(outsidePath, linkDirPath) != 0) {
|
||||
result = -851;
|
||||
goto cleanup;
|
||||
}
|
||||
linkDirDone = 1;
|
||||
|
||||
/* WMKDIR returns EEXIST for the existing symlink; the callback must
|
||||
* refuse to chdir through it rather than escape basePath */
|
||||
ret = wsScpRecvCallback(ssh, WOLFSSH_SCP_NEW_DIR, basePath,
|
||||
"linkdir", 0755, 0, 0, 0, NULL, 0, 0, NULL);
|
||||
if (ret != WS_SCP_ABORT) {
|
||||
result = -852;
|
||||
goto cleanup;
|
||||
}
|
||||
if (ssh->scpDirDepth != 0) {
|
||||
result = -853;
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
/* plant a (dangling) file symlink pointing outside basePath */
|
||||
if (symlink(leakedPath, linkFilePath) != 0) {
|
||||
result = -854;
|
||||
goto cleanup;
|
||||
}
|
||||
linkFileDone = 1;
|
||||
|
||||
/* the callback must refuse to open the symlink rather than write through
|
||||
* it to the outside target */
|
||||
ret = wsScpRecvCallback(ssh, WOLFSSH_SCP_NEW_FILE, basePath,
|
||||
"linkfile", 0644, 0, 0, 0, NULL, 0, 0, NULL);
|
||||
if (ret != WS_SCP_ABORT) {
|
||||
result = -855;
|
||||
goto cleanup;
|
||||
}
|
||||
if (stat(leakedPath, &st) == 0) {
|
||||
(void)remove(leakedPath);
|
||||
result = -856;
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
cleanup:
|
||||
if (ssh != NULL)
|
||||
wolfSSH_free(ssh);
|
||||
if (ctx != NULL)
|
||||
wolfSSH_CTX_free(ctx);
|
||||
free(basePath);
|
||||
/* NEW_REQUEST changed the process CWD into basePath, so leave it before
|
||||
* removing the created directories or the rmdir calls would fail */
|
||||
if (origCwdSaved && chdir(origCwd) != 0 && result == 0)
|
||||
result = -857;
|
||||
if (linkDirDone)
|
||||
(void)remove(linkDirPath);
|
||||
if (linkFileDone)
|
||||
(void)remove(linkFilePath);
|
||||
if (outsideMkdirDone)
|
||||
(void)rmdir(outsidePath);
|
||||
if (baseMkdirDone)
|
||||
(void)rmdir(basePathRaw);
|
||||
(void)rmdir(tmpDir);
|
||||
return result;
|
||||
#endif /* WOLFSSH_HAVE_SYMLINK */
|
||||
}
|
||||
|
||||
#endif /* WOLFSSH_SCP recv callback depth guard test */
|
||||
|
||||
|
||||
|
|
@ -4977,6 +5145,11 @@ int wolfSSH_UnitTest(int argc, char** argv)
|
|||
printf("ScpRecvCallback_NewDirChdirFail: %s\n",
|
||||
(unitResult == 0 ? "SUCCESS" : "FAILED"));
|
||||
testResult = testResult || unitResult;
|
||||
|
||||
unitResult = test_ScpRecvCallback_SymlinkGuard();
|
||||
printf("ScpRecvCallback_SymlinkGuard: %s\n",
|
||||
(unitResult == 0 ? "SUCCESS" : "FAILED"));
|
||||
testResult = testResult || unitResult;
|
||||
#endif
|
||||
|
||||
#ifdef WOLFSSH_TEST_CAPTURING_ALLOCATOR
|
||||
|
|
|
|||
|
|
@ -1531,6 +1531,15 @@ extern "C" {
|
|||
#define WOLFSSH_HAVE_SYMLINK
|
||||
#endif
|
||||
|
||||
/* wIsSymlink lives in the always-compiled port.c, but its filesystem
|
||||
* dependencies (WSTAT_T/WLSTAT/S_ISLNK on POSIX, WS_GetFileAttributesExA on
|
||||
* Windows) and its only callers exist solely in the SFTP and SCP code, so
|
||||
* gate it on those features in addition to the platform capability. */
|
||||
#if defined(WOLFSSH_HAVE_SYMLINK) && \
|
||||
(defined(WOLFSSH_SFTP) || defined(WOLFSSH_SCP))
|
||||
WOLFSSH_LOCAL int wIsSymlink(const char* path);
|
||||
#endif
|
||||
|
||||
#ifndef WS_MAYBE_UNUSED
|
||||
#if (defined(__GNUC__) && (__GNUC__ >= 4)) || defined(__clang__) || \
|
||||
defined(__IAR_SYSTEMS_ICC__)
|
||||
|
|
|
|||
Loading…
Reference in New Issue