Author: Sam McCall Date: 2021-06-08T21:30:01-07:00 New Revision: 84e8b1cf07b9845ea8e1e07ed5ccc3c5a70d975b
URL: https://github.com/llvm/llvm-project/commit/84e8b1cf07b9845ea8e1e07ed5ccc3c5a70d975b DIFF: https://github.com/llvm/llvm-project/commit/84e8b1cf07b9845ea8e1e07ed5ccc3c5a70d975b.diff LOG: [clangd] Only allow remote index to be enabled from user config. Differential Revision: https://reviews.llvm.org/D100542 (cherry picked from commit ecf93a716c9ecf2e38898547df90323e239a623c) Added: Modified: clang-tools-extra/clangd/ConfigCompile.cpp clang-tools-extra/clangd/ConfigFragment.h clang-tools-extra/clangd/ConfigProvider.cpp clang-tools-extra/clangd/ConfigProvider.h clang-tools-extra/clangd/tool/ClangdMain.cpp clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index b4f0d61868863..7d5466778a81d 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -101,6 +101,7 @@ struct FragmentCompiler { llvm::SourceMgr *SourceMgr; // Normalized Fragment::SourceInfo::Directory. std::string FragmentDirectory; + bool Trusted = false; llvm::Optional<llvm::Regex> compileRegex(const Located<std::string> &Text, @@ -183,6 +184,7 @@ struct FragmentCompiler { } void compile(Fragment &&F) { + Trusted = F.Source.Trusted; if (!F.Source.Directory.empty()) { FragmentDirectory = llvm::sys::path::convert_to_slash(F.Source.Directory); if (FragmentDirectory.back() != '/') @@ -319,6 +321,13 @@ struct FragmentCompiler { void compile(Fragment::IndexBlock::ExternalBlock &&External, llvm::SMRange BlockRange) { + if (External.Server && !Trusted) { + diag(Error, + "Remote index may not be specified by untrusted configuration. " + "Copy this into user config to use it.", + External.Server->Range); + return; + } #ifndef CLANGD_ENABLE_REMOTE if (External.Server) { elog("Clangd isn't compiled with remote index support, ignoring Server: " @@ -489,8 +498,8 @@ CompiledFragment Fragment::compile(DiagnosticCallback D) && { trace::Span Tracer("ConfigCompile"); SPAN_ATTACH(Tracer, "ConfigFile", ConfigFile); auto Result = std::make_shared<CompiledFragmentImpl>(); - vlog("Config fragment: compiling {0}:{1} -> {2}", ConfigFile, LineCol.first, - Result.get()); + vlog("Config fragment: compiling {0}:{1} -> {2} (trusted={3})", ConfigFile, + LineCol.first, Result.get(), Source.Trusted); FragmentCompiler{*Result, D, Source.Manager.get()}.compile(std::move(*this)); // Return as cheaply-copyable wrapper. diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index c36b07f5e8e22..c98ca3a2dd522 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -94,6 +94,9 @@ struct Fragment { /// Absolute path to directory the fragment is associated with. Relative /// paths mentioned in the fragment are resolved against this. std::string Directory; + /// Whether this fragment is allowed to make critical security/privacy + /// decisions. + bool Trusted = false; }; SourceInfo Source; diff --git a/clang-tools-extra/clangd/ConfigProvider.cpp b/clang-tools-extra/clangd/ConfigProvider.cpp index 05b2ba50566dd..6dfb00b14fc6c 100644 --- a/clang-tools-extra/clangd/ConfigProvider.cpp +++ b/clang-tools-extra/clangd/ConfigProvider.cpp @@ -34,7 +34,7 @@ class FileConfigCache : public FileCache { : FileCache(Path), Directory(Directory) {} void get(const ThreadsafeFS &TFS, DiagnosticCallback DC, - std::chrono::steady_clock::time_point FreshTime, + std::chrono::steady_clock::time_point FreshTime, bool Trusted, std::vector<CompiledFragment> &Out) const { read( TFS, FreshTime, @@ -43,6 +43,7 @@ class FileConfigCache : public FileCache { if (Data) for (auto &Fragment : Fragment::parseYAML(*Data, path(), DC)) { Fragment.Source.Directory = Directory; + Fragment.Source.Trusted = Trusted; CachedValue.push_back(std::move(Fragment).compile(DC)); } }, @@ -52,35 +53,38 @@ class FileConfigCache : public FileCache { std::unique_ptr<Provider> Provider::fromYAMLFile(llvm::StringRef AbsPath, llvm::StringRef Directory, - const ThreadsafeFS &FS) { + const ThreadsafeFS &FS, + bool Trusted) { class AbsFileProvider : public Provider { mutable FileConfigCache Cache; // threadsafe const ThreadsafeFS &FS; + bool Trusted; std::vector<CompiledFragment> getFragments(const Params &P, DiagnosticCallback DC) const override { std::vector<CompiledFragment> Result; - Cache.get(FS, DC, P.FreshTime, Result); + Cache.get(FS, DC, P.FreshTime, Trusted, Result); return Result; }; public: AbsFileProvider(llvm::StringRef Path, llvm::StringRef Directory, - const ThreadsafeFS &FS) - : Cache(Path, Directory), FS(FS) { + const ThreadsafeFS &FS, bool Trusted) + : Cache(Path, Directory), FS(FS), Trusted(Trusted) { assert(llvm::sys::path::is_absolute(Path)); } }; - return std::make_unique<AbsFileProvider>(AbsPath, Directory, FS); + return std::make_unique<AbsFileProvider>(AbsPath, Directory, FS, Trusted); } std::unique_ptr<Provider> Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath, - const ThreadsafeFS &FS) { + const ThreadsafeFS &FS, bool Trusted) { class RelFileProvider : public Provider { std::string RelPath; const ThreadsafeFS &FS; + bool Trusted; mutable std::mutex Mu; // Keys are the (posix-style) ancestor directory, not the config within it. @@ -128,18 +132,19 @@ Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath, // This will take a (per-file) lock for each file that actually exists. std::vector<CompiledFragment> Result; for (FileConfigCache *Cache : Caches) - Cache->get(FS, DC, P.FreshTime, Result); + Cache->get(FS, DC, P.FreshTime, Trusted, Result); return Result; }; public: - RelFileProvider(llvm::StringRef RelPath, const ThreadsafeFS &FS) - : RelPath(RelPath), FS(FS) { + RelFileProvider(llvm::StringRef RelPath, const ThreadsafeFS &FS, + bool Trusted) + : RelPath(RelPath), FS(FS), Trusted(Trusted) { assert(llvm::sys::path::is_relative(RelPath)); } }; - return std::make_unique<RelFileProvider>(RelPath, FS); + return std::make_unique<RelFileProvider>(RelPath, FS, Trusted); } std::unique_ptr<Provider> diff --git a/clang-tools-extra/clangd/ConfigProvider.h b/clang-tools-extra/clangd/ConfigProvider.h index 25d9450f28a72..428438b67f14d 100644 --- a/clang-tools-extra/clangd/ConfigProvider.h +++ b/clang-tools-extra/clangd/ConfigProvider.h @@ -69,7 +69,8 @@ class Provider { /// Directory will be used to resolve relative paths in the fragments. static std::unique_ptr<Provider> fromYAMLFile(llvm::StringRef AbsPath, llvm::StringRef Directory, - const ThreadsafeFS &); + const ThreadsafeFS &, + bool Trusted = false); // Reads fragments from YAML files found relative to ancestors of Params.Path. // // All fragments that exist are returned, starting from distant ancestors. @@ -78,7 +79,8 @@ class Provider { // // If Params does not specify a path, no fragments are returned. static std::unique_ptr<Provider> - fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath, const ThreadsafeFS &); + fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath, const ThreadsafeFS &, + bool Trusted = false); /// A provider that includes fragments from all the supplied providers. /// Order is preserved; later providers take precedence over earlier ones. diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index fe69079bfe67c..99c3d97ce35d3 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -831,8 +831,8 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var if (llvm::sys::path::user_config_directory(UserConfig)) { llvm::sys::path::append(UserConfig, "clangd", "config.yaml"); vlog("User config file is {0}", UserConfig); - ProviderStack.push_back( - config::Provider::fromYAMLFile(UserConfig, /*Directory=*/"", TFS)); + ProviderStack.push_back(config::Provider::fromYAMLFile( + UserConfig, /*Directory=*/"", TFS, /*Trusted=*/true)); } else { elog("Couldn't determine user config file, not loading"); } diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp index 4961d3474fd9a..a3738681bec5a 100644 --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -318,7 +318,21 @@ TEST_F(ConfigCompileTests, TidyBadChecks) { DiagKind(llvm::SourceMgr::DK_Warning)))); } +TEST_F(ConfigCompileTests, ExternalServerNeedsTrusted) { + Fragment::IndexBlock::ExternalBlock External; + External.Server.emplace("xxx"); + Frag.Index.External = std::move(External); + compileAndApply(); + EXPECT_THAT( + Diags.Diagnostics, + ElementsAre(DiagMessage( + "Remote index may not be specified by untrusted configuration. " + "Copy this into user config to use it."))); + EXPECT_FALSE(Conf.Index.External.hasValue()); +} + TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) { + Frag.Source.Trusted = true; Fragment::IndexBlock::ExternalBlock External; External.File.emplace(""); External.Server.emplace(""); _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
