Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1da49dc9 authored by Mårten Kongstad's avatar Mårten Kongstad Committed by Todd Kennedy
Browse files

idmap2: lock down write access to /data/resouce-cache 

Deny write access to /data/resource-cache for UIDs other than root and
system. While this is already handled by SELinux rules, add an
additional layer of security to explicitly prevent malicious apps from
messing with the system's idmap files.

Test: make idmap2_tests
Change-Id: Id986633558d5d02452276f05f64337a8700f148a
parent 793f1a79

cmds/idmap2/idmap2/Create.cpp

+8 −0
Original line number Diff line number Diff line
@@ -39,6 +39,7 @@ using android::idmap2::PolicyBitmask; 
using android::idmap2::PolicyFlags;

using android::idmap2::Result;

using android::idmap2::utils::kIdmapFilePermissionMask;

using android::idmap2::utils::UidHasWriteAccessToPath;



bool Create(const std::vector<std::string>& args, std::ostream& out_error) {

  std::string target_apk_path;
@@ -66,6 +67,13 @@ bool Create(const std::vector<std::string>& args, std::ostream& out_error) { 
    return false;

  }



  const uid_t uid = getuid();

  if (!UidHasWriteAccessToPath(uid, idmap_path)) {

    out_error << "error: uid " << uid << " does not have write access to " << idmap_path

              << std::endl;

    return false;

  }



  PolicyBitmask fulfilled_policies = 0;

  if (auto result = PoliciesToBitmask(policies, out_error)) {

    fulfilled_policies |= *result;

cmds/idmap2/idmap2d/Idmap2Service.cpp

+17 −3
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ 


#include "android-base/macros.h"

#include "android-base/stringprintf.h"

#include "binder/IPCThreadState.h"

#include "utils/String8.h"

#include "utils/Trace.h"
@@ -38,18 +39,19 @@ 


#include "idmap2d/Idmap2Service.h"



using android::IPCThreadState;

using android::binder::Status;

using android::idmap2::BinaryStreamVisitor;

using android::idmap2::Idmap;

using android::idmap2::IdmapHeader;

using android::idmap2::PolicyBitmask;

using android::idmap2::Result;

using android::idmap2::utils::kIdmapCacheDir;

using android::idmap2::utils::kIdmapFilePermissionMask;

using android::idmap2::utils::UidHasWriteAccessToPath;



namespace {



constexpr const char* kIdmapCacheDir = "/data/resource-cache";



Status ok() {

  return Status::ok();

}
@@ -77,7 +79,13 @@ Status Idmap2Service::getIdmapPath(const std::string& overlay_apk_path, 
Status Idmap2Service::removeIdmap(const std::string& overlay_apk_path,

                                  int32_t user_id ATTRIBUTE_UNUSED, bool* _aidl_return) {

  assert(_aidl_return);

  const uid_t uid = IPCThreadState::self()->getCallingUid();

  const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);

  if (!UidHasWriteAccessToPath(uid, idmap_path)) {

    *_aidl_return = false;

    return error(base::StringPrintf("failed to unlink %s: calling uid %d lacks write access",

                                    idmap_path.c_str(), uid));

  }

  if (unlink(idmap_path.c_str()) != 0) {

    *_aidl_return = false;

    return error("failed to unlink " + idmap_path + ": " + strerror(errno));
@@ -118,6 +126,13 @@ Status Idmap2Service::createIdmap(const std::string& target_apk_path, 


  const PolicyBitmask policy_bitmask = ConvertAidlArgToPolicyBitmask(fulfilled_policies);



  const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);

  const uid_t uid = IPCThreadState::self()->getCallingUid();

  if (!UidHasWriteAccessToPath(uid, idmap_path)) {

    return error(base::StringPrintf("will not write to %s: calling uid %d lacks write accesss",

                                    idmap_path.c_str(), uid));

  }



  const std::unique_ptr<const ApkAssets> target_apk = ApkAssets::Load(target_apk_path);

  if (!target_apk) {

    return error("failed to load apk " + target_apk_path);
@@ -137,7 +152,6 @@ Status Idmap2Service::createIdmap(const std::string& target_apk_path, 
  }



  umask(kIdmapFilePermissionMask);

  const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);

  std::ofstream fout(idmap_path);

  if (fout.fail()) {

    return error("failed to open idmap path " + idmap_path);

cmds/idmap2/include/idmap2/FileUtils.h

+5 −0
Original line number Diff line number Diff line
@@ -17,6 +17,8 @@ 
#ifndef IDMAP2_INCLUDE_IDMAP2_FILEUTILS_H_

#define IDMAP2_INCLUDE_IDMAP2_FILEUTILS_H_



#include <sys/types.h>



#include <functional>

#include <memory>

#include <string>
@@ -24,6 +26,7 @@ 


namespace android::idmap2::utils {



constexpr const char* kIdmapCacheDir = "/data/resource-cache";

constexpr const mode_t kIdmapFilePermissionMask = 0133;  // u=rw,g=r,o=r



typedef std::function<bool(unsigned char type /* DT_* from dirent.h */, const std::string& path)>
@@ -35,6 +38,8 @@ std::unique_ptr<std::string> ReadFile(int fd); 


std::unique_ptr<std::string> ReadFile(const std::string& path);



bool UidHasWriteAccessToPath(uid_t uid, const std::string& path);



}  // namespace android::idmap2::utils



#endif  // IDMAP2_INCLUDE_IDMAP2_FILEUTILS_H_

cmds/idmap2/libidmap2/FileUtils.cpp

+30 −0
Original line number Diff line number Diff line
@@ -19,12 +19,20 @@ 
#include <unistd.h>



#include <cerrno>

#include <climits>

#include <cstdlib>

#include <cstring>

#include <fstream>

#include <memory>

#include <string>

#include <utility>

#include <vector>



#include "android-base/file.h"

#include "android-base/macros.h"

#include "android-base/stringprintf.h"

#include "private/android_filesystem_config.h"



#include "idmap2/FileUtils.h"



namespace android::idmap2::utils {
@@ -77,4 +85,26 @@ std::unique_ptr<std::string> ReadFile(int fd) { 
  return r == 0 ? std::move(str) : nullptr;

}



#ifdef __ANDROID__

bool UidHasWriteAccessToPath(uid_t uid, const std::string& path) {

  // resolve symlinks and relative paths; the directories must exist

  std::string canonical_path;

  if (!base::Realpath(base::Dirname(path), &canonical_path)) {

    return false;

  }



  const std::string cache_subdir = base::StringPrintf("%s/", kIdmapCacheDir);

  if (canonical_path == kIdmapCacheDir ||

      canonical_path.compare(0, cache_subdir.size(), cache_subdir) == 0) {

    // limit access to /data/resource-cache to root and system

    return uid == AID_ROOT || uid == AID_SYSTEM;

  }

  return true;

}

#else

bool UidHasWriteAccessToPath(uid_t uid ATTRIBUTE_UNUSED, const std::string& path ATTRIBUTE_UNUSED) {

  return true;

}

#endif



}  // namespace android::idmap2::utils

cmds/idmap2/tests/FileUtilsTests.cpp

+23 −0
Original line number Diff line number Diff line
@@ -22,6 +22,8 @@ 
#include "gtest/gtest.h"



#include "android-base/macros.h"

#include "android-base/stringprintf.h"

#include "private/android_filesystem_config.h"



#include "idmap2/FileUtils.h"
@@ -71,4 +73,25 @@ TEST(FileUtilsTests, ReadFile) { 
  close(pipefd[0]);

}



#ifdef __ANDROID__

TEST(FileUtilsTests, UidHasWriteAccessToPath) {

  constexpr const char* tmp_path = "/data/local/tmp/test@idmap";

  const std::string cache_path(base::StringPrintf("%s/test@idmap", kIdmapCacheDir));

  const std::string sneaky_cache_path(base::StringPrintf("/data/../%s/test@idmap", kIdmapCacheDir));



  ASSERT_TRUE(UidHasWriteAccessToPath(AID_ROOT, tmp_path));

  ASSERT_TRUE(UidHasWriteAccessToPath(AID_ROOT, cache_path));

  ASSERT_TRUE(UidHasWriteAccessToPath(AID_ROOT, sneaky_cache_path));



  ASSERT_TRUE(UidHasWriteAccessToPath(AID_SYSTEM, tmp_path));

  ASSERT_TRUE(UidHasWriteAccessToPath(AID_SYSTEM, cache_path));

  ASSERT_TRUE(UidHasWriteAccessToPath(AID_SYSTEM, sneaky_cache_path));



  constexpr const uid_t AID_SOME_APP = AID_SYSTEM + 1;

  ASSERT_TRUE(UidHasWriteAccessToPath(AID_SOME_APP, tmp_path));

  ASSERT_FALSE(UidHasWriteAccessToPath(AID_SOME_APP, cache_path));

  ASSERT_FALSE(UidHasWriteAccessToPath(AID_SOME_APP, sneaky_cache_path));

}

#endif



}  // namespace android::idmap2::utils