Verified Commit 01f14f72 authored by Jed Davis's avatar Jed Davis Committed by ma1
Browse files

Bug 2067976 - Clean up some unnecessary string copies in SandboxBroker. a=diannaS DONTBUILD 

Original Revision: https://phabricator.services.mozilla.com/D326377

Differential Revision: https://phabricator.services.mozilla.com/D328249
parent 99d3b149

security/sandbox/linux/broker/SandboxBroker.cpp

+10 −17
Changes for security/sandbox/linux/broker/SandboxBroker.cpp: 10 added lines, 17 removed lines.
Original line number Diff line number Diff line
@@ -575,29 +575,22 @@ nsCString SandboxBroker::ReverseSymlinks(const nsACString& aPath) { 
  }

}



int SandboxBroker::SymlinkPermissions(const char* aPath,

                                      const size_t aPathLen) {

  // Work on a temporary copy, so we can reverse it.

  // Because we bail on a writable dir, SymlinkPath

  // might not restore the callers' path exactly.

  char pathBufSymlink[kMaxPathLen + 1];

  strcpy(pathBufSymlink, aPath);



  nsCString orig =

      ReverseSymlinks(nsDependentCString(pathBufSymlink, aPathLen));

  if (!orig.IsEmpty()) {

int SandboxBroker::SymlinkPermissions(const nsACString& aPath) {

  nsCString path = ReverseSymlinks(aPath);

  if (path.IsEmpty()) {

    path = aPath;

  } else {

    if (SandboxInfo::Get().Test(SandboxInfo::kVerbose)) {

      SANDBOX_LOG("Reversing %s -> %s", aPath, orig.get());

      SANDBOX_LOG("Reversing %s -> %s", PromiseFlatCString(aPath).get(), path.get());

    }

    base::strlcpy(pathBufSymlink, orig.get(), sizeof(pathBufSymlink));

  }



  int perms = 0;

  // Resolve relative paths, propagate permissions and

  // fail if a symlink is in a writable path. The output is in perms.

  char* result =

      SandboxBroker::SymlinkPath(mPolicy.get(), pathBufSymlink, NULL, &perms);

  if (result != NULL) {

  char* result = SandboxBroker::SymlinkPath(mPolicy.get(), path.get(),

                                            nullptr, &perms);

  if (result != nullptr) {

    free(result);

    // We finished the translation, so we have a usable return in "perms".

    return perms;
@@ -747,7 +740,7 @@ void SandboxBroker::ThreadMain(void) { 
        // Then try to figure out the original path and see if that is

        // readable. Work on the original path, this reverses

        // ConvertRelative above.

        int symlinkPerms = SymlinkPermissions(recvBuf, first_len);

        int symlinkPerms = SymlinkPermissions(nsDependentCString(recvBuf, first_len));

        if (symlinkPerms > 0) {

          perms = symlinkPerms;

        }

security/sandbox/linux/broker/SandboxBroker.h

+1 −1
Changes for security/sandbox/linux/broker/SandboxBroker.h: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -163,7 +163,7 @@ class SandboxBroker final : private SandboxBrokerCommon, 
  size_t RealPath(char* aPath, size_t aBufSize, size_t aPathLen);

  nsCString ReverseSymlinks(const nsACString& aPath);

  // Retrieves permissions for the path the original symlink sits in.

  int SymlinkPermissions(const char* aPath, const size_t aPathLen);

  int SymlinkPermissions(const nsACString& aPath);

  // In SandboxBrokerRealPath.cpp

  char* SymlinkPath(const Policy* aPolicy, const char* __restrict aPath,

                    char* __restrict aResolved, int* aPermission);