Skip to content

Commit

Permalink
util/getOr: prevent trivial use-after-free
Browse files Browse the repository at this point in the history
  • Loading branch information
fogti committed May 4, 2022
1 parent 971ba33 commit e5722a4
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 21 deletions.
6 changes: 3 additions & 3 deletions src/libexpr/flake/flakeref.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment(
parsedURL.query.insert_or_assign("shallow", "1");

return std::make_pair(
FlakeRef(Input::fromURL(parsedURL), getOr(parsedURL.query, "dir", "")),
FlakeRef(Input::fromURL(parsedURL), Path(getOr(parsedURL.query, "dir"))),
fragment);
}

Expand All @@ -189,7 +189,7 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment(
if (!hasPrefix(path, "/"))
throw BadURL("flake reference '%s' is not an absolute path", url);
auto query = decodeQuery(match[2]);
path = canonPath(path + "/" + getOr(query, "dir", ""));
path = canonPath(fmt("%s/%s", path, getOr(query, "dir")));
}

fetchers::Attrs attrs;
Expand All @@ -208,7 +208,7 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment(
input.parent = baseDir;

return std::make_pair(
FlakeRef(std::move(input), getOr(parsedURL.query, "dir", "")),
FlakeRef(std::move(input), Path(getOr(parsedURL.query, "dir"))),
fragment);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/get-drvs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ DrvInfo::DrvInfo(EvalState & state, ref<Store> store, const std::string & drvPat

outputName =
selectedOutputs.empty()
? getOr(drv.env, "outputName", "out")
: *selectedOutputs.begin();
? getOr(drv.env, "outputName", std::string_view("out"))
: std::string_view(*selectedOutputs.begin());

auto i = drv.outputs.find(outputName);
if (i == drv.outputs.end())
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/build/local-derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ void LocalDerivationGoal::startBuilder()
temporary build directory. The text files have the format used
by `nix-store --register-validity'. However, the deriver
fields are left empty. */
auto s = getOr(drv->env, "exportReferencesGraph", "");
auto s = getOr(drv->env, "exportReferencesGraph");
Strings ss = tokenizeString<Strings>(s);
if (ss.size() % 2 != 0)
throw BuildError("odd number of tokens in 'exportReferencesGraph': '%1%'", s);
Expand Down Expand Up @@ -988,7 +988,7 @@ void LocalDerivationGoal::initTmpDir() {
there is no size constraint). */
if (!parsedDrv->getStructuredAttrs()) {

StringSet passAsFile = tokenizeString<StringSet>(getOr(drv->env, "passAsFile", ""));
StringSet passAsFile = tokenizeString<StringSet>(getOr(drv->env, "passAsFile"));
for (auto & i : drv->env) {
if (passAsFile.find(i.first) == passAsFile.end()) {
env[i.first] = i.second;
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/builtins/fetchurl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData)

Path storePath = getAttr("out");
auto mainUrl = getAttr("url");
bool unpack = getOr(drv.env, "unpack", "") == "1";
bool unpack = getOr(drv.env, "unpack") == "1";

/* Note: have to use a fresh fileTransfer here because we're in
a forked process. */
Expand Down
8 changes: 4 additions & 4 deletions src/libstore/filetransfer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -692,10 +692,10 @@ struct curlFileTransfer : public FileTransfer
#if ENABLE_S3
auto [bucketName, key, params] = parseS3Uri(request.uri);

std::string profile = getOr(params, "profile", "");
std::string region = getOr(params, "region", Aws::Region::US_EAST_1);
std::string scheme = getOr(params, "scheme", "");
std::string endpoint = getOr(params, "endpoint", "");
std::string profile(getOr(params, "profile"));
std::string region(getOr(params, "region", std::string_view(Aws::Region::US_EAST_1)));
std::string scheme(getOr(params, "scheme"));
std::string endpoint(getOr(params, "endpoint"));

S3Helper s3Helper(profile, region, scheme, endpoint);

Expand Down
4 changes: 2 additions & 2 deletions src/libstore/store-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1314,8 +1314,8 @@ static bool isNonUriPath(const std::string & spec) {
std::shared_ptr<Store> openFromNonUri(const std::string & uri, const Store::Params & params)
{
if (uri == "" || uri == "auto") {
auto stateDir = getOr(params, "state", settings.nixStateDir);
if (access(stateDir.c_str(), R_OK | W_OK) == 0)
auto stateDir = getOr(params, "state", std::string_view(settings.nixStateDir));
if (access(stateDir.data(), R_OK | W_OK) == 0)
return std::make_shared<LocalStore>(params);
else if (pathExists(settings.nixDaemonSocketFile))
return std::make_shared<UDSRemoteStore>(params);
Expand Down
4 changes: 2 additions & 2 deletions src/libutil/tests/tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ namespace nix {
StringMap s = { };
auto expected = "yi";

ASSERT_EQ(getOr(s, "one", "yi"), expected);
ASSERT_EQ(getOr(s, "one", std::string_view("yi")), expected);
}

TEST(getOr, getFromContainer) {
Expand All @@ -575,7 +575,7 @@ namespace nix {
s["two"] = "er";
auto expected = "yi";

ASSERT_EQ(getOr(s, "one", "nope"), expected);
ASSERT_EQ(getOr(s, "one", std::string_view("nope")), expected);
}

/* ----------------------------------------------------------------------------
Expand Down
7 changes: 4 additions & 3 deletions src/libutil/util.hh
Original file line number Diff line number Diff line change
Expand Up @@ -559,10 +559,11 @@ typename T::mapped_type * get(T & map, const typename T::key_type & key)
}

/* Get a value for the specified key from an associate container, or a default value if the key isn't present. */
template <class T>
const typename T::mapped_type & getOr(T & map,
// NOTE: we restrict the value to string_views to make sure that use-after-free doesn't happen trivially
template <class T, class CharT = typename T::mapped_type::value_type>
std::basic_string_view<CharT> getOr(T & map,
const typename T::key_type & key,
const typename T::mapped_type & defaultValue)
const std::basic_string_view<CharT> defaultValue = std::basic_string_view<CharT>())
{
auto i = map.find(key);
if (i == map.end()) return defaultValue;
Expand Down
2 changes: 1 addition & 1 deletion src/nix-build/nix-build.cc
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ static void main_nix_build(int argc, char * * argv)
env["NIX_STORE"] = store->storeDir;
env["NIX_BUILD_CORES"] = std::to_string(settings.buildCores);

auto passAsFile = tokenizeString<StringSet>(getOr(drv.env, "passAsFile", ""));
auto passAsFile = tokenizeString<StringSet>(getOr(drv.env, "passAsFile"));

bool keepTmp = false;
int fileNr = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ int main(int argc, char ** argv)
impurePaths.insert(argv[2]);
else {
auto drv = store->derivationFromPath(store->parseStorePath(argv[1]));
impurePaths = tokenizeString<StringSet>(getOr(drv.env, "__impureHostDeps", ""));
impurePaths = tokenizeString<StringSet>(getOr(drv.env, "__impureHostDeps"));
impurePaths.insert("/usr/lib/libSystem.dylib");
}

Expand Down

0 comments on commit e5722a4

Please sign in to comment.