mirror of
https://github.com/NixOS/nix.git
synced 2025-11-08 19:46:02 +01:00
Remove validation of URLs passed to FileTransferRequest verbatim
CURL is not very strict about validation of URLs passed to it. We should reflect this in our handling of URLs that we get from the user in <nix/fetchurl.nix> or builtins.fetchurl. ValidURL was an attempt to rectify this, but it turned out to be too strict. The only good way to resolve this is to pass (in some cases) the user-provided string verbatim to CURL. Other usages in libfetchers still benefit from using structured ParsedURL and validation though. nix store prefetch-file --name foo 'https://cdn.skypack.dev/big.js@^5.2.2' error: 'https://cdn.skypack.dev/big.js@^5.2.2' is not a valid URL: leftover
This commit is contained in:
parent
0f85ef3677
commit
47f427a172
7 changed files with 44 additions and 37 deletions
|
|
@ -42,7 +42,7 @@ DownloadFileResult downloadFile(
|
||||||
if (cached && !cached->expired)
|
if (cached && !cached->expired)
|
||||||
return useCached();
|
return useCached();
|
||||||
|
|
||||||
FileTransferRequest request(ValidURL{url});
|
FileTransferRequest request(VerbatimURL{url});
|
||||||
request.headers = headers;
|
request.headers = headers;
|
||||||
if (cached)
|
if (cached)
|
||||||
request.expectedETag = getStrAttr(cached->value, "etag");
|
request.expectedETag = getStrAttr(cached->value, "etag");
|
||||||
|
|
@ -107,13 +107,13 @@ DownloadFileResult downloadFile(
|
||||||
static DownloadTarballResult downloadTarball_(
|
static DownloadTarballResult downloadTarball_(
|
||||||
const Settings & settings, const std::string & urlS, const Headers & headers, const std::string & displayPrefix)
|
const Settings & settings, const std::string & urlS, const Headers & headers, const std::string & displayPrefix)
|
||||||
{
|
{
|
||||||
ValidURL url = urlS;
|
ParsedURL url = parseURL(urlS);
|
||||||
|
|
||||||
// Some friendly error messages for common mistakes.
|
// Some friendly error messages for common mistakes.
|
||||||
// Namely lets catch when the url is a local file path, but
|
// Namely lets catch when the url is a local file path, but
|
||||||
// it is not in fact a tarball.
|
// it is not in fact a tarball.
|
||||||
if (url.scheme() == "file") {
|
if (url.scheme == "file") {
|
||||||
std::filesystem::path localPath = renderUrlPathEnsureLegal(url.path());
|
std::filesystem::path localPath = renderUrlPathEnsureLegal(url.path);
|
||||||
if (!exists(localPath)) {
|
if (!exists(localPath)) {
|
||||||
throw Error("tarball '%s' does not exist.", localPath);
|
throw Error("tarball '%s' does not exist.", localPath);
|
||||||
}
|
}
|
||||||
|
|
@ -164,7 +164,7 @@ static DownloadTarballResult downloadTarball_(
|
||||||
|
|
||||||
/* Note: if the download is cached, `importTarball()` will receive
|
/* Note: if the download is cached, `importTarball()` will receive
|
||||||
no data, which causes it to import an empty tarball. */
|
no data, which causes it to import an empty tarball. */
|
||||||
auto archive = !url.path().empty() && hasSuffix(toLower(url.path().back()), ".zip") ? ({
|
auto archive = !url.path.empty() && hasSuffix(toLower(url.path.back()), ".zip") ? ({
|
||||||
/* In streaming mode, libarchive doesn't handle
|
/* In streaming mode, libarchive doesn't handle
|
||||||
symlinks in zip files correctly (#10649). So write
|
symlinks in zip files correctly (#10649). So write
|
||||||
the entire file to disk so libarchive can access it
|
the entire file to disk so libarchive can access it
|
||||||
|
|
@ -178,7 +178,7 @@ static DownloadTarballResult downloadTarball_(
|
||||||
}
|
}
|
||||||
TarArchive{path};
|
TarArchive{path};
|
||||||
})
|
})
|
||||||
: TarArchive{*source};
|
: TarArchive{*source};
|
||||||
auto tarballCache = getTarballCache();
|
auto tarballCache = getTarballCache();
|
||||||
auto parseSink = tarballCache->getFileSystemObjectSink();
|
auto parseSink = tarballCache->getFileSystemObjectSink();
|
||||||
auto lastModified = unpackTarfileToSink(archive, *parseSink);
|
auto lastModified = unpackTarfileToSink(archive, *parseSink);
|
||||||
|
|
|
||||||
|
|
@ -37,7 +37,7 @@ static void builtinFetchurl(const BuiltinBuilderContext & ctx)
|
||||||
|
|
||||||
auto fetch = [&](const std::string & url) {
|
auto fetch = [&](const std::string & url) {
|
||||||
auto source = sinkToSource([&](Sink & sink) {
|
auto source = sinkToSource([&](Sink & sink) {
|
||||||
FileTransferRequest request(ValidURL{url});
|
FileTransferRequest request(VerbatimURL{url});
|
||||||
request.decompress = false;
|
request.decompress = false;
|
||||||
|
|
||||||
auto decompressor = makeDecompressionSink(unpack && hasSuffix(mainUrl, ".xz") ? "xz" : "none", sink);
|
auto decompressor = makeDecompressionSink(unpack && hasSuffix(mainUrl, ".xz") ? "xz" : "none", sink);
|
||||||
|
|
|
||||||
|
|
@ -95,7 +95,7 @@ struct UsernameAuth
|
||||||
|
|
||||||
struct FileTransferRequest
|
struct FileTransferRequest
|
||||||
{
|
{
|
||||||
ValidURL uri;
|
VerbatimURL uri;
|
||||||
Headers headers;
|
Headers headers;
|
||||||
std::string expectedETag;
|
std::string expectedETag;
|
||||||
bool verifyTLS = true;
|
bool verifyTLS = true;
|
||||||
|
|
@ -121,7 +121,7 @@ struct FileTransferRequest
|
||||||
std::optional<std::string> preResolvedAwsSessionToken;
|
std::optional<std::string> preResolvedAwsSessionToken;
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
FileTransferRequest(ValidURL uri)
|
FileTransferRequest(VerbatimURL uri)
|
||||||
: uri(std::move(uri))
|
: uri(std::move(uri))
|
||||||
, parentAct(getCurActivity())
|
, parentAct(getCurActivity())
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,9 @@
|
||||||
|
|
||||||
#include "nix/util/error.hh"
|
#include "nix/util/error.hh"
|
||||||
#include "nix/util/canon-path.hh"
|
#include "nix/util/canon-path.hh"
|
||||||
|
#include "nix/util/split.hh"
|
||||||
|
#include "nix/util/util.hh"
|
||||||
|
#include "nix/util/variant-wrapper.hh"
|
||||||
|
|
||||||
namespace nix {
|
namespace nix {
|
||||||
|
|
||||||
|
|
@ -342,8 +345,7 @@ ParsedURL fixGitURL(const std::string & url);
|
||||||
bool isValidSchemeName(std::string_view scheme);
|
bool isValidSchemeName(std::string_view scheme);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Either a ParsedURL or a verbatim string, but the string must be a valid
|
* Either a ParsedURL or a verbatim string. This is necessary because in certain cases URI must be passed
|
||||||
* ParsedURL. This is necessary because in certain cases URI must be passed
|
|
||||||
* verbatim (e.g. in builtin fetchers), since those are specified by the user.
|
* verbatim (e.g. in builtin fetchers), since those are specified by the user.
|
||||||
* In those cases normalizations performed by the ParsedURL might be surprising
|
* In those cases normalizations performed by the ParsedURL might be surprising
|
||||||
* and undesirable, since Nix must be a universal client that has to work with
|
* and undesirable, since Nix must be a universal client that has to work with
|
||||||
|
|
@ -354,23 +356,23 @@ bool isValidSchemeName(std::string_view scheme);
|
||||||
*
|
*
|
||||||
* Though we perform parsing and validation for internal needs.
|
* Though we perform parsing and validation for internal needs.
|
||||||
*/
|
*/
|
||||||
struct ValidURL : private ParsedURL
|
struct VerbatimURL
|
||||||
{
|
{
|
||||||
std::optional<std::string> encoded;
|
using Raw = std::variant<std::string, ParsedURL>;
|
||||||
|
Raw raw;
|
||||||
|
|
||||||
ValidURL(std::string str)
|
VerbatimURL(std::string_view s)
|
||||||
: ParsedURL(parseURL(str, /*lenient=*/false))
|
: raw(std::string{s})
|
||||||
, encoded(std::move(str))
|
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
ValidURL(std::string_view str)
|
VerbatimURL(std::string s)
|
||||||
: ValidURL(std::string{str})
|
: raw(std::move(s))
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
ValidURL(ParsedURL parsed)
|
VerbatimURL(ParsedURL url)
|
||||||
: ParsedURL{std::move(parsed)}
|
: raw(std::move(url))
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -379,25 +381,35 @@ struct ValidURL : private ParsedURL
|
||||||
*/
|
*/
|
||||||
std::string to_string() const
|
std::string to_string() const
|
||||||
{
|
{
|
||||||
return encoded.or_else([&]() -> std::optional<std::string> { return ParsedURL::to_string(); }).value();
|
return std::visit(
|
||||||
|
overloaded{
|
||||||
|
[](const std::string & str) { return str; }, [](const ParsedURL & url) { return url.to_string(); }},
|
||||||
|
raw);
|
||||||
}
|
}
|
||||||
|
|
||||||
const ParsedURL & parsed() const &
|
const ParsedURL parsed() const
|
||||||
{
|
{
|
||||||
return *this;
|
return std::visit(
|
||||||
|
overloaded{
|
||||||
|
[](const std::string & str) { return parseURL(str); }, [](const ParsedURL & url) { return url; }},
|
||||||
|
raw);
|
||||||
}
|
}
|
||||||
|
|
||||||
std::string_view scheme() const &
|
std::string_view scheme() const &
|
||||||
{
|
{
|
||||||
return ParsedURL::scheme;
|
return std::visit(
|
||||||
}
|
overloaded{
|
||||||
|
[](std::string_view str) {
|
||||||
const auto & path() const &
|
auto scheme = splitPrefixTo(str, ':');
|
||||||
{
|
if (!scheme)
|
||||||
return ParsedURL::path;
|
throw BadURL("URL '%s' doesn't have a scheme", str);
|
||||||
|
return *scheme;
|
||||||
|
},
|
||||||
|
[](const ParsedURL & url) -> std::string_view { return url.scheme; }},
|
||||||
|
raw);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
std::ostream & operator<<(std::ostream & os, const ValidURL & url);
|
std::ostream & operator<<(std::ostream & os, const VerbatimURL & url);
|
||||||
|
|
||||||
} // namespace nix
|
} // namespace nix
|
||||||
|
|
|
||||||
|
|
@ -434,7 +434,7 @@ bool isValidSchemeName(std::string_view s)
|
||||||
return std::regex_match(s.begin(), s.end(), regex, std::regex_constants::match_default);
|
return std::regex_match(s.begin(), s.end(), regex, std::regex_constants::match_default);
|
||||||
}
|
}
|
||||||
|
|
||||||
std::ostream & operator<<(std::ostream & os, const ValidURL & url)
|
std::ostream & operator<<(std::ostream & os, const VerbatimURL & url)
|
||||||
{
|
{
|
||||||
os << url.to_string();
|
os << url.to_string();
|
||||||
return os;
|
return os;
|
||||||
|
|
|
||||||
|
|
@ -105,7 +105,7 @@ std::tuple<StorePath, Hash> prefetchFile(
|
||||||
|
|
||||||
FdSink sink(fd.get());
|
FdSink sink(fd.get());
|
||||||
|
|
||||||
FileTransferRequest req(ValidURL{url});
|
FileTransferRequest req(VerbatimURL{url});
|
||||||
req.decompress = false;
|
req.decompress = false;
|
||||||
getFileTransfer()->download(std::move(req), sink);
|
getFileTransfer()->download(std::move(req), sink);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -88,8 +88,3 @@ requireDaemonNewerThan "2.20"
|
||||||
expected=100
|
expected=100
|
||||||
if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly
|
if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly
|
||||||
expectStderr $expected nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url "file://$narxz" 2>&1 | grep 'must be a fixed-output or impure derivation'
|
expectStderr $expected nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url "file://$narxz" 2>&1 | grep 'must be a fixed-output or impure derivation'
|
||||||
|
|
||||||
requireDaemonNewerThan "2.32.0pre20250831"
|
|
||||||
|
|
||||||
expect 1 nix-build --expr 'import <nix/fetchurl.nix>' --argstr name 'name' --argstr url "file://authority.not.allowed/fetchurl.sh?a=1&a=2" --no-out-link |&
|
|
||||||
grepQuiet "error: file:// URL 'file://authority.not.allowed/fetchurl.sh?a=1&a=2' has unexpected authority 'authority.not.allowed'"
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue