From patchwork Thu Jan 18 01:55:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Victoria Dye X-Patchwork-Id: 13522305 Received: from mail-wr1-f42.google.com (mail-wr1-f42.google.com [209.85.221.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AB8D12115 for ; Thu, 18 Jan 2024 01:55:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.42 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705542924; cv=none; b=DTaRHIDmYDfHxtDhUvb7WIbD7C66UrTLDjcTYQ4lBNDXsn5OsTyRTXV/TTKSPtSk+eP0kLr8WT7VwyFxPaqjY1Omb7BueD9NbZGti2+TU5KKK0bLup5x35RlnV/iI8aXNcLUAUeDRIjY/lQqeEhtwo0vfhPta9vPwxyRqaQ3OWM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705542924; c=relaxed/simple; bh=02goR1oUIRqaAX6wkSZ6kwyZ0o5zwu0LRjNURjZXLhA=; h=Received:DKIM-Signature:X-Google-DKIM-Signature: X-Gm-Message-State:X-Google-Smtp-Source:X-Received:Received: Message-ID:In-Reply-To:References:From:Date:Subject:Fcc: Content-Type:Content-Transfer-Encoding:MIME-Version:To:Cc; b=C4izqzBJ/TAzRyFRpf1S5OlCjLaDta0htyENLj4NyUztJR3s8w+LmVe7sq1P42A9G45NzpDYaGOCBBnR50a1g/A9NXk1kzKxAHqj/NFUC1oYCLlyv7BJ87F5gaG5VTHG8ilVB8ky/Psy+Ip6nY+A3KLrtV0p2x0E/izPb6hvdmY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=DSKyO38p; arc=none smtp.client-ip=209.85.221.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="DSKyO38p" Received: by mail-wr1-f42.google.com with SMTP id ffacd0b85a97d-337d05b8942so79328f8f.3 for ; Wed, 17 Jan 2024 17:55:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705542920; x=1706147720; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=DvQKSY9TRfwOJuLTaN1VfXvLnXYDZbW+dUt2kJaOjdg=; b=DSKyO38pJ/9ZUKlAMEJwpZa6IZQqjVq0WPCFBnSj8kJnBnxHbV2yphhl13XRFVxE/E N+LmLHPCVevqf9vVFb1PvKK7jIa3gpFdKTx5h32qMmoAi2xClA5kimdUDoXcRmc94YsU DG5TIYYFMf+PvZBAXF7SrRHFRy37IfuToKENgHnWsmB6DLwTfPy38l2pt8ocHblvhle9 kpTezMHs/GSSzC+KT1V6uzaZnEv0+2FINlxb8aX5gD3LCvjH12T85szm1GRf0E9EOgje tO77EijYQKwIRA7mZfbKuia7ecrDRpJVCZnNqK7USItCaAWngMUq8RE9HU9fn9yaEOvN KtcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705542920; x=1706147720; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=DvQKSY9TRfwOJuLTaN1VfXvLnXYDZbW+dUt2kJaOjdg=; b=MTVAO1p0XFmzSXRFkhNKlwUa3oqrL03i84PpgTH1cInEev/yET0nC4Kebw/NNhicfQ i7/KEcB3vc76NaEFltrM1pZ491cLinPE65p1+t/6IVVIOAX/tZREoSW8bnNDO+lxrApu /cESpm7O8LGIGa6cgIkpA1rs9vVtyvVjkxqtFxt6t3UzblGSdh4O+iOi5Jj9pGriq7+G 3ezqBcyaqlKoqlYRsRvKaFPGGl31MTIq7sCjxer9aPZBv4LsEAwwJUiXs1Jx+t2XbOFb UQagKEVN6f6psMJzo87BwBoGJlK/HGRO+hzU8J7ljUgACYiS7KckSzs6pKU13rGynM6m HiNQ== X-Gm-Message-State: AOJu0Ywcwbj9X6EhWPK230wvOOF3k7pWbA1nf6Igzf+1XaYZw+LyYpkn pjFj1ABEzRWNDYpRbuVPv0maP96PzrUPMxlK1hs7gmVBNa3v69TbJgLeE95F X-Google-Smtp-Source: AGHT+IEjdqcp9FDFsokuj68qFe7wpgB+K8vDeunv77IRs1G7A0TQQw8bIwBj4EKPH0RCWxPzP1Qofg== X-Received: by 2002:a05:600c:168a:b0:40e:8000:3180 with SMTP id k10-20020a05600c168a00b0040e80003180mr57283wmn.50.1705542920301; Wed, 17 Jan 2024 17:55:20 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ay39-20020a05600c1e2700b0040e92521b24sm75657wmb.30.2024.01.17.17.55.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jan 2024 17:55:20 -0800 (PST) Message-ID: In-Reply-To: References: Date: Thu, 18 Jan 2024 01:55:15 +0000 Subject: [PATCH v2 1/4] submodule-config.h: move check_submodule_url Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Patrick Steinhardt , Jeff King , Victoria Dye , Victoria Dye From: Victoria Dye From: Victoria Dye Move 'check_submodule_url' out of 'fsck.c' and into 'submodule-config.h' as a public method, similar to 'check_submodule_name'. With the function now accessible outside of 'fsck', it can be used in a later commit to extend 'test-tool submodule' to check the validity of submodule URLs as it does with names in the 'check-name' subcommand. Other than its location, no changes are made to 'check_submodule_url' in this patch. Signed-off-by: Victoria Dye --- fsck.c | 133 -------------------------------------------- submodule-config.c | 134 +++++++++++++++++++++++++++++++++++++++++++++ submodule-config.h | 3 + 3 files changed, 137 insertions(+), 133 deletions(-) diff --git a/fsck.c b/fsck.c index 1ad02fcdfab..8ded0a473a4 100644 --- a/fsck.c +++ b/fsck.c @@ -20,7 +20,6 @@ #include "packfile.h" #include "submodule-config.h" #include "config.h" -#include "credential.h" #include "help.h" static ssize_t max_tree_entry_len = 4096; @@ -1047,138 +1046,6 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer, return ret; } -static int starts_with_dot_slash(const char *const path) -{ - return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_SLASH | - PATH_MATCH_XPLATFORM); -} - -static int starts_with_dot_dot_slash(const char *const path) -{ - return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH | - PATH_MATCH_XPLATFORM); -} - -static int submodule_url_is_relative(const char *url) -{ - return starts_with_dot_slash(url) || starts_with_dot_dot_slash(url); -} - -/* - * Count directory components that a relative submodule URL should chop - * from the remote_url it is to be resolved against. - * - * In other words, this counts "../" components at the start of a - * submodule URL. - * - * Returns the number of directory components to chop and writes a - * pointer to the next character of url after all leading "./" and - * "../" components to out. - */ -static int count_leading_dotdots(const char *url, const char **out) -{ - int result = 0; - while (1) { - if (starts_with_dot_dot_slash(url)) { - result++; - url += strlen("../"); - continue; - } - if (starts_with_dot_slash(url)) { - url += strlen("./"); - continue; - } - *out = url; - return result; - } -} -/* - * Check whether a transport is implemented by git-remote-curl. - * - * If it is, returns 1 and writes the URL that would be passed to - * git-remote-curl to the "out" parameter. - * - * Otherwise, returns 0 and leaves "out" untouched. - * - * Examples: - * http::https://example.com/repo.git -> 1, https://example.com/repo.git - * https://example.com/repo.git -> 1, https://example.com/repo.git - * git://example.com/repo.git -> 0 - * - * This is for use in checking for previously exploitable bugs that - * required a submodule URL to be passed to git-remote-curl. - */ -static int url_to_curl_url(const char *url, const char **out) -{ - /* - * We don't need to check for case-aliases, "http.exe", and so - * on because in the default configuration, is_transport_allowed - * prevents URLs with those schemes from being cloned - * automatically. - */ - if (skip_prefix(url, "http::", out) || - skip_prefix(url, "https::", out) || - skip_prefix(url, "ftp::", out) || - skip_prefix(url, "ftps::", out)) - return 1; - if (starts_with(url, "http://") || - starts_with(url, "https://") || - starts_with(url, "ftp://") || - starts_with(url, "ftps://")) { - *out = url; - return 1; - } - return 0; -} - -static int check_submodule_url(const char *url) -{ - const char *curl_url; - - if (looks_like_command_line_option(url)) - return -1; - - if (submodule_url_is_relative(url) || starts_with(url, "git://")) { - char *decoded; - const char *next; - int has_nl; - - /* - * This could be appended to an http URL and url-decoded; - * check for malicious characters. - */ - decoded = url_decode(url); - has_nl = !!strchr(decoded, '\n'); - - free(decoded); - if (has_nl) - return -1; - - /* - * URLs which escape their root via "../" can overwrite - * the host field and previous components, resolving to - * URLs like https::example.com/submodule.git and - * https:///example.com/submodule.git that were - * susceptible to CVE-2020-11008. - */ - if (count_leading_dotdots(url, &next) > 0 && - (*next == ':' || *next == '/')) - return -1; - } - - else if (url_to_curl_url(url, &curl_url)) { - struct credential c = CREDENTIAL_INIT; - int ret = 0; - if (credential_from_url_gently(&c, curl_url, 1) || - !*c.host) - ret = -1; - credential_clear(&c); - return ret; - } - - return 0; -} - struct fsck_gitmodules_data { const struct object_id *oid; struct fsck_options *options; diff --git a/submodule-config.c b/submodule-config.c index f4dd482abc9..3b295e9f89c 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -14,6 +14,8 @@ #include "parse-options.h" #include "thread-utils.h" #include "tree-walk.h" +#include "url.h" +#include "credential.h" /* * submodule cache lookup structure @@ -228,6 +230,138 @@ int check_submodule_name(const char *name) return 0; } +static int starts_with_dot_slash(const char *const path) +{ + return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_SLASH | + PATH_MATCH_XPLATFORM); +} + +static int starts_with_dot_dot_slash(const char *const path) +{ + return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH | + PATH_MATCH_XPLATFORM); +} + +static int submodule_url_is_relative(const char *url) +{ + return starts_with_dot_slash(url) || starts_with_dot_dot_slash(url); +} + +/* + * Count directory components that a relative submodule URL should chop + * from the remote_url it is to be resolved against. + * + * In other words, this counts "../" components at the start of a + * submodule URL. + * + * Returns the number of directory components to chop and writes a + * pointer to the next character of url after all leading "./" and + * "../" components to out. + */ +static int count_leading_dotdots(const char *url, const char **out) +{ + int result = 0; + while (1) { + if (starts_with_dot_dot_slash(url)) { + result++; + url += strlen("../"); + continue; + } + if (starts_with_dot_slash(url)) { + url += strlen("./"); + continue; + } + *out = url; + return result; + } +} +/* + * Check whether a transport is implemented by git-remote-curl. + * + * If it is, returns 1 and writes the URL that would be passed to + * git-remote-curl to the "out" parameter. + * + * Otherwise, returns 0 and leaves "out" untouched. + * + * Examples: + * http::https://example.com/repo.git -> 1, https://example.com/repo.git + * https://example.com/repo.git -> 1, https://example.com/repo.git + * git://example.com/repo.git -> 0 + * + * This is for use in checking for previously exploitable bugs that + * required a submodule URL to be passed to git-remote-curl. + */ +static int url_to_curl_url(const char *url, const char **out) +{ + /* + * We don't need to check for case-aliases, "http.exe", and so + * on because in the default configuration, is_transport_allowed + * prevents URLs with those schemes from being cloned + * automatically. + */ + if (skip_prefix(url, "http::", out) || + skip_prefix(url, "https::", out) || + skip_prefix(url, "ftp::", out) || + skip_prefix(url, "ftps::", out)) + return 1; + if (starts_with(url, "http://") || + starts_with(url, "https://") || + starts_with(url, "ftp://") || + starts_with(url, "ftps://")) { + *out = url; + return 1; + } + return 0; +} + +int check_submodule_url(const char *url) +{ + const char *curl_url; + + if (looks_like_command_line_option(url)) + return -1; + + if (submodule_url_is_relative(url) || starts_with(url, "git://")) { + char *decoded; + const char *next; + int has_nl; + + /* + * This could be appended to an http URL and url-decoded; + * check for malicious characters. + */ + decoded = url_decode(url); + has_nl = !!strchr(decoded, '\n'); + + free(decoded); + if (has_nl) + return -1; + + /* + * URLs which escape their root via "../" can overwrite + * the host field and previous components, resolving to + * URLs like https::example.com/submodule.git and + * https:///example.com/submodule.git that were + * susceptible to CVE-2020-11008. + */ + if (count_leading_dotdots(url, &next) > 0 && + (*next == ':' || *next == '/')) + return -1; + } + + else if (url_to_curl_url(url, &curl_url)) { + struct credential c = CREDENTIAL_INIT; + int ret = 0; + if (credential_from_url_gently(&c, curl_url, 1) || + !*c.host) + ret = -1; + credential_clear(&c); + return ret; + } + + return 0; +} + static int name_and_item_from_var(const char *var, struct strbuf *name, struct strbuf *item) { diff --git a/submodule-config.h b/submodule-config.h index 958f320ac6c..b6133af71b0 100644 --- a/submodule-config.h +++ b/submodule-config.h @@ -89,6 +89,9 @@ int config_set_in_gitmodules_file_gently(const char *key, const char *value); */ int check_submodule_name(const char *name); +/* Returns 0 if the URL valid per RFC3986 and -1 otherwise. */ +int check_submodule_url(const char *url); + /* * Note: these helper functions exist solely to maintain backward * compatibility with 'fetch' and 'update_clone' storing configuration in From patchwork Thu Jan 18 01:55:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Victoria Dye X-Patchwork-Id: 13522306 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 21AFA23D9 for ; Thu, 18 Jan 2024 01:55:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.45 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705542925; cv=none; b=ghDwX5fzYZi5Pv7HmO+MU9CCHjxbkpmJVTQXvD/ac6jPUxs+s/QmLf202FnPoXl8YMSMSetxmn4na+W5fZgZiO1ngY6XTZ6XMuVjsZLmMl3uLdhjILLmUdjhMO/qINJIDtuJvhCR75GHS+218m2Mf6eDBP0OFy0Sr+t6rh07vrM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705542925; c=relaxed/simple; bh=0zMrQDGmuy4bMai58DNJDUKmgYfn6EnW4nMWR+P3t0Q=; h=Received:DKIM-Signature:X-Google-DKIM-Signature: X-Gm-Message-State:X-Google-Smtp-Source:X-Received:Received: Message-ID:In-Reply-To:References:From:Date:Subject:Fcc: Content-Type:Content-Transfer-Encoding:MIME-Version:To:Cc; b=Df8DJMWYK2V3q0mPnf6hfmDy2a85gveEiB7sYdJYhZcmLjuckLTikAaxVdBiOCUbn4tB43Xzt/xMIcbT5UiQW33cB+Kqk1BAdn83S09Avl5SAtm0Ob3VCny8j1rkrFipQbte9FDwvsr23YEX9zlJTBOCOxHLO3nAqFCPfeEseaY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=PaqZA+zT; arc=none smtp.client-ip=209.85.128.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="PaqZA+zT" Received: by mail-wm1-f45.google.com with SMTP id 5b1f17b1804b1-40e8ff22382so1610725e9.1 for ; Wed, 17 Jan 2024 17:55:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705542922; x=1706147722; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=+ZkrPlXxi/jWsPrKBdM5fZ76oDS6iXeSRVY8TxOzMIY=; b=PaqZA+zT1rucLXldXLxuwRaA1/M/P8hUiXP+ZAigj7HG3SB/hzgWwdjcpqeHAMqo9G jjyoTZRSuaV/oH8FevE/M8JRk5WQATCJCU3UX8MuF3l5FbqF7jajm8OAMvjHw3206TS3 8junfd4GA7ApbrsKdJbw+jp8gHyaNXPyl0bbEyklDSd6jam6ThqEuaZ9lt9GVo0LBomZ FNkVfxSaHK5PzTmfOUIToj5B1gqpllQhbkJ2EPFMWrWS4HG2JQ08yu7RvWdq3Cqzv9+n NZQ81at0AmsYAmQs3Dhk6YfN1jLlm3XU6/ECzjlTua5DtoZEjWJLzLGkS8/FJeLY1H6V AUGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705542922; x=1706147722; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+ZkrPlXxi/jWsPrKBdM5fZ76oDS6iXeSRVY8TxOzMIY=; b=wQ1sjjWY2Err+P6+Jzny4ZnnUlxM5NDrlF9QmU2d7+9zt+DgDaZT2sc1c4Ct16GhyX 9sNSFA57jxIf6E0/dlpsImv/VGZzQxBOi6LKRDzNcHAULWahy1btbERkCgnmuC34Wa+Y mZk6I1VIzywrM26Tf88VxlTHZ8bge0T81HS4C+G8JsncHbaki4vVaR8dLdgNU8C89/22 hzcK8nx3xtoYZ9Bz5kXZLsOnfY5aw3C8opML/Vt47lS16RKlm6wvdYhRXRYdarmxZGvR XDutD2hXBaRbT8PFmpiTROZkEASRQvzqP5KN7i/GXBMQX1Iz528wr62U555BFI9BGEOC bxhA== X-Gm-Message-State: AOJu0YyXZw9JeRniNpT+GpxX3yNqK1bsCcJLxubQ0dqxvZp7v5g/BChH qssuhknPNflNDyCkbf/M7WZTRlFybtETTgT4T0rtWbRNwIRi81Bw+CTbvMlC X-Google-Smtp-Source: AGHT+IHio9d7m2g0xAi1sWtgFKDzN5YvDP/ZDtzomPX+WyhrdAtQSVbJlIR5lhdNqCJPlDInY2RkRw== X-Received: by 2002:a05:600c:2205:b0:40e:8613:848 with SMTP id z5-20020a05600c220500b0040e86130848mr49081wml.117.1705542921640; Wed, 17 Jan 2024 17:55:21 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id hg14-20020a05600c538e00b0040e5cf9a6c7sm24416098wmb.13.2024.01.17.17.55.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jan 2024 17:55:20 -0800 (PST) Message-ID: <14e8834c38bcddc21856772b09f6fa77fa924b48.1705542918.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 18 Jan 2024 01:55:16 +0000 Subject: [PATCH v2 2/4] test-submodule: remove command line handling for check-name Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Patrick Steinhardt , Jeff King , Victoria Dye , Victoria Dye From: Victoria Dye From: Victoria Dye The 'check-name' subcommand to 'test-tool submodule' is documented as being able to take a command line argument ''. However, this does not work - and has never worked - because 'argc > 0' triggers the usage message in 'cmd__submodule_check_name()'. To simplify the helper and avoid future confusion around proper use of the subcommand, remove any references to command line arguments for 'check-name' in usage strings and handling in 'check_name()'. Helped-by: Jeff King Signed-off-by: Victoria Dye --- t/helper/test-submodule.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c index 50c154d0370..9adbc8d1568 100644 --- a/t/helper/test-submodule.c +++ b/t/helper/test-submodule.c @@ -9,7 +9,7 @@ #include "submodule.h" #define TEST_TOOL_CHECK_NAME_USAGE \ - "test-tool submodule check-name " + "test-tool submodule check-name" static const char *submodule_check_name_usage[] = { TEST_TOOL_CHECK_NAME_USAGE, NULL @@ -36,26 +36,15 @@ static const char *submodule_usage[] = { NULL }; -/* - * Exit non-zero if any of the submodule names given on the command line is - * invalid. If no names are given, filter stdin to print only valid names - * (which is primarily intended for testing). - */ -static int check_name(int argc, const char **argv) +/* Filter stdin to print only valid names. */ +static int check_name(void) { - if (argc > 1) { - while (*++argv) { - if (check_submodule_name(*argv) < 0) - return 1; - } - } else { - struct strbuf buf = STRBUF_INIT; - while (strbuf_getline(&buf, stdin) != EOF) { - if (!check_submodule_name(buf.buf)) - printf("%s\n", buf.buf); - } - strbuf_release(&buf); + struct strbuf buf = STRBUF_INIT; + while (strbuf_getline(&buf, stdin) != EOF) { + if (!check_submodule_name(buf.buf)) + printf("%s\n", buf.buf); } + strbuf_release(&buf); return 0; } @@ -69,7 +58,7 @@ static int cmd__submodule_check_name(int argc, const char **argv) if (argc) usage_with_options(submodule_check_name_usage, options); - return check_name(argc, argv); + return check_name(); } static int cmd__submodule_is_active(int argc, const char **argv) From patchwork Thu Jan 18 01:55:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Victoria Dye X-Patchwork-Id: 13522307 Received: from mail-wm1-f46.google.com (mail-wm1-f46.google.com [209.85.128.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 540DB2573 for ; Thu, 18 Jan 2024 01:55:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705542926; cv=none; b=hG+n8tSVcnWM0VjB8Th5x+Z6RJ7kmSOzS2BbhTMfCJN4gTP0odSEG/SkxYYq1neFX/2C/yo+Z268yH2/8gBoGPpJto8053j58b5ZLFaSZ6F31sTtVPYQByMptH88tEpHIXkAMMgTU+2y74Ki4lltU8d5/jOBa1cWJcL9PmrHLpw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705542926; c=relaxed/simple; bh=GB+N8gFzH4vTKIGCtmd+GCGSDkAcUu72ECNFI39JLaE=; h=Received:DKIM-Signature:X-Google-DKIM-Signature: X-Gm-Message-State:X-Google-Smtp-Source:X-Received:Received: Message-ID:In-Reply-To:References:From:Date:Subject:Fcc: Content-Type:Content-Transfer-Encoding:MIME-Version:To:Cc; b=t7vTeJiWur2FJ3dfA0Y84VrUcK03asX/TLuexi1kc1v4GibR7W/JmlTuHaS3aoGyUPjCtEPx773E6Uy7v8HXKiNtbqT4ZLnkIz4rjblkLxGGcgmwJXypGr8ZODcd37XxQ2yXPS/P6vMUkl0i+pjrs9iFpQzb0mYUm02bk0i8AoY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=BqClTPp6; arc=none smtp.client-ip=209.85.128.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="BqClTPp6" Received: by mail-wm1-f46.google.com with SMTP id 5b1f17b1804b1-40e8d3b32eeso9106895e9.1 for ; Wed, 17 Jan 2024 17:55:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705542922; x=1706147722; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=PavoUQAhhD9XkWJ25IHDPp+eG0+ToT+MytcUOPj1yQ8=; b=BqClTPp6skMhP/ebt7zMlirPyrf0D2ySl6oO6jetmv6s8Kmbuj61GZyQ4I5Yr89WHO vIi7G/GfZAZkysfOKv/uKxFGHPpOtIoSR1hhIQBs0LI2ixeU/pxcygKATG0f8AA2cmXA qABya11NbMe3duOGxJSLPDsdPPpW1PXYtgGuEUkf66HI0by5009NvZzc8hKLpCBelIeY q3mNTKBqQKdEzYVruYQwGOziggyZbBTnq4U1P3yFOolVIvBn0iX1CpCLoLZbc2pNclj6 67gPJGgtAT8WfO6A3q8xTZMYmbHv1ESUMlbuNtz2yGHNXlkSnAUYlBd3FRSMb/Ya1k2j 0cmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705542922; x=1706147722; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PavoUQAhhD9XkWJ25IHDPp+eG0+ToT+MytcUOPj1yQ8=; b=i/7519uXLyrlk4GirAcd/s6r3/HjjR6QFBLlj1qiYroxUXiV8u3nfKGXG5btn7wZPP nnpHUQoo4+TJRoLAHopd6wPTPL8B3DiNPxz4ABJfrMarY7qrKKm3H8TY0OuI77w8B0qa rDu+MOQK6Oqg6YtH6j8fnGpwX8cm78TiOpI1d5kQryYJXni71m+UJhhZkvrYym5/zwDu o2O7JW4SBWBsbhjf9jNOY+e9Id1teY+/fxBCwLMG4kWnM8iZsKBBBKYaznPF4Oc0HLOy fWBkYi5kM/rPOk9tinzVjsR2ClQFYW7Yqa3CUAgUv4KAdqk0iMPqVpj5PaVH7prGv5T6 +oSA== X-Gm-Message-State: AOJu0Yw+h4nzUW1BTi06J9Y83JK3ZXmeiCljRzK08/CgbTOvzaWbg1Zu DPfc5Avzt62DL2qwkhurQ4Z0W9xIdVoRCp79eA3lZNkGwgKiW82sn9a9WYiV X-Google-Smtp-Source: AGHT+IHcba0Cml9zUwMS/xFjex7/mPazZhhBke7Lvvephm22myVo05VaOBJiFJT7WRrCESxxokyRvA== X-Received: by 2002:a05:600c:2151:b0:40e:3cb0:b27c with SMTP id v17-20020a05600c215100b0040e3cb0b27cmr52827wml.71.1705542922181; Wed, 17 Jan 2024 17:55:22 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id iv17-20020a05600c549100b0040e5034d8e0sm28210933wmb.43.2024.01.17.17.55.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jan 2024 17:55:21 -0800 (PST) Message-ID: In-Reply-To: References: Date: Thu, 18 Jan 2024 01:55:17 +0000 Subject: [PATCH v2 3/4] t7450: test submodule urls Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Patrick Steinhardt , Jeff King , Victoria Dye , Victoria Dye From: Victoria Dye From: Victoria Dye Add tests to 't7450-bad-git-dotfiles.sh' to check the validity of different submodule URLs. To verify this directly (without setting up test repositories & submodules), add a 'check-url' subcommand to 'test-tool submodule' that calls 'check_submodule_url' in the same way that 'check-name' calls 'check_submodule_name'. Add two tests to separately address cases where the URL check correctly filters out invalid URLs and cases where the check misses invalid URLs. Mark the latter ("url check misses invalid cases") with 'test_expect_failure' to indicate that this not the undesired behavior. Signed-off-by: Victoria Dye --- t/helper/test-submodule.c | 35 +++++++++++++++++++++++++++++++---- t/t7450-bad-git-dotfiles.sh | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c index 9adbc8d1568..7197969a081 100644 --- a/t/helper/test-submodule.c +++ b/t/helper/test-submodule.c @@ -15,6 +15,13 @@ static const char *submodule_check_name_usage[] = { NULL }; +#define TEST_TOOL_CHECK_URL_USAGE \ + "test-tool submodule check-url" +static const char *submodule_check_url_usage[] = { + TEST_TOOL_CHECK_URL_USAGE, + NULL +}; + #define TEST_TOOL_IS_ACTIVE_USAGE \ "test-tool submodule is-active " static const char *submodule_is_active_usage[] = { @@ -31,17 +38,23 @@ static const char *submodule_resolve_relative_url_usage[] = { static const char *submodule_usage[] = { TEST_TOOL_CHECK_NAME_USAGE, + TEST_TOOL_CHECK_URL_USAGE, TEST_TOOL_IS_ACTIVE_USAGE, TEST_TOOL_RESOLVE_RELATIVE_URL_USAGE, NULL }; -/* Filter stdin to print only valid names. */ -static int check_name(void) +typedef int (*check_fn_t)(const char *); + +/* + * Apply 'check_fn' to each line of stdin, printing values that pass the check + * to stdout. + */ +static int check_submodule(check_fn_t check_fn) { struct strbuf buf = STRBUF_INIT; while (strbuf_getline(&buf, stdin) != EOF) { - if (!check_submodule_name(buf.buf)) + if (!check_fn(buf.buf)) printf("%s\n", buf.buf); } strbuf_release(&buf); @@ -58,7 +71,20 @@ static int cmd__submodule_check_name(int argc, const char **argv) if (argc) usage_with_options(submodule_check_name_usage, options); - return check_name(); + return check_submodule(check_submodule_name); +} + +static int cmd__submodule_check_url(int argc, const char **argv) +{ + struct option options[] = { + OPT_END() + }; + argc = parse_options(argc, argv, "test-tools", options, + submodule_check_url_usage, 0); + if (argc) + usage_with_options(submodule_check_url_usage, options); + + return check_submodule(check_submodule_url); } static int cmd__submodule_is_active(int argc, const char **argv) @@ -184,6 +210,7 @@ static int cmd__submodule_config_writeable(int argc, const char **argv UNUSED) static struct test_cmd cmds[] = { { "check-name", cmd__submodule_check_name }, + { "check-url", cmd__submodule_check_url }, { "is-active", cmd__submodule_is_active }, { "resolve-relative-url", cmd__submodule_resolve_relative_url}, { "config-list", cmd__submodule_config_list }, diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index 35a31acd4d7..c73b1c92ecc 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -45,6 +45,41 @@ test_expect_success 'check names' ' test_cmp expect actual ' +test_expect_success 'check urls' ' + cat >expect <<-\EOF && + ./bar/baz/foo.git + https://example.com/foo.git + http://example.com:80/deeper/foo.git + EOF + + test-tool submodule check-url >actual <<-\EOF && + ./bar/baz/foo.git + https://example.com/foo.git + http://example.com:80/deeper/foo.git + -a./foo + ../../..//test/foo.git + ../../../../../:localhost:8080/foo.git + ..\../.\../:example.com/foo.git + ./%0ahost=example.com/foo.git + https://one.example.com/evil?%0ahost=two.example.com + https:///example.com/foo.git + https::example.com/foo.git + http:::example.com/foo.git + EOF + + test_cmp expect actual +' + +# NEEDSWORK: the URL checked here is not valid (and will not work as a remote if +# a user attempts to clone it), but the fsck check passes. +test_expect_failure 'url check misses invalid cases' ' + test-tool submodule check-url >actual <<-\EOF && + http://example.com:test/foo.git + EOF + + test_must_be_empty actual +' + test_expect_success 'create innocent subrepo' ' git init innocent && git -C innocent commit --allow-empty -m foo From patchwork Thu Jan 18 01:55:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Victoria Dye X-Patchwork-Id: 13522308 Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D892F2907 for ; Thu, 18 Jan 2024 01:55:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705542926; cv=none; b=KLRIfAn6NPSvChKt8OEDa2Ha4aPSsn2otzhpMGsSK9sMWZu8SyWvH5C4AM6ejyN9wNCKlg2ehKRmpD381+4XITDyo1wS0X/uhLciRCVknd0etr88NSiANztV0FCB9Ok7dIr4db+R43js8+tHRo19hQnCXON5yJxlaU4poMHCF1g= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705542926; c=relaxed/simple; bh=3NNrQZpl/NffJJraBIZAsKYiBiMV6KKZNyx3HcAHqrk=; h=Received:DKIM-Signature:X-Google-DKIM-Signature: X-Gm-Message-State:X-Google-Smtp-Source:X-Received:Received: Message-ID:In-Reply-To:References:From:Date:Subject:Fcc: Content-Type:Content-Transfer-Encoding:MIME-Version:To:Cc; b=vGQTPKtpxyRwt6elr5cCjCURBbY1X8pMgc/rO89zCiTaT/rXWk+pTR+rSiCAx5w5Sp0K75L6F2/nVReSp/nt9GWmMeaLCO2U/TeLONWZ8lhpiOsa+fJuCD6b2gre7eBSPI2A/rIaj6gd7q+ujO//zigfhVt9b1xwAtswDZDIXXA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ipoF3ypQ; arc=none smtp.client-ip=209.85.221.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ipoF3ypQ" Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-337bea8682dso1729376f8f.0 for ; Wed, 17 Jan 2024 17:55:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705542923; x=1706147723; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=dxW2/4NI8M1WRQA7E9YTtofRKir2GzW3+QI5y4KHHBs=; b=ipoF3ypQvJ3oPXrs4XbAs/2V7jyLe4yAylLxAkvPq2aqCbZSdfiUzGfrxJWh+rt09i VX9tAxMvbsxo2XZ1Tw834vXPNP+KiUBqMBYG7Ti+nje41KGg8NdLR4xxaBeYWzk2WlYr l/vKqeFnmdctj5EPuhxqBDA7M4A3NBAFBvK/Snzv+c/hKdkOtR9OSW9f57uFr8cd33Dt O/RgqXZyV4Y7Z+FihPq4vETaJwrtRWdJgI2F2bWYo2nDPxdQfz4Co4YZ2L3DO/mF7qCA ZBIbpHLlDPH5YvXlqH0rDsEr0sidzUX+aVwYZ1gKpOSHOOrO61f9NAkhSolI+QgLqIMK a0qg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705542923; x=1706147723; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=dxW2/4NI8M1WRQA7E9YTtofRKir2GzW3+QI5y4KHHBs=; b=DHVzO2MFbXjKH4xfFuSmQd/MXI6msh02iKXEmC32tNyFL5HZHUw5LDGl7qzr3DzYbi 5Z0SEYaUMs1IatsRgZiICrdSDVgvdyHW8i/FqocuZ0xkSAM7jfc5/NbITsJyz1vHxO3j AEwtbPBF+GZoF7cG8qgc1Iy0h9uKbcfj8fFpX+Ghw1JbtUv6plBGdNpbo21Wv/uyXUxp m3MWhRTImpZarQWZ4U09cbRQ/JPwugQNYcA/7kVlx0AhajdREcil8Ehe9sBsnf/OvIuj hx0xQ0oDJam8xxm/w2mFcya7E60OMiZq4hr4lwWMU9kZcLIQHZXFdTSt6zVXCdZHi/+j b5Mg== X-Gm-Message-State: AOJu0YwYyRApzNLeVnDG/yWDFjb4nVxR1XyKj1cEuxjZywcfeJM/PxZa M5D6QpmZBLvTbdRCJkYaxEuOdGF3aoiAIV54KErXzVljlr7zhIV/55DU6FwT X-Google-Smtp-Source: AGHT+IE2BOYBTJmsWCU7nzxhsylmFthElVy+XRrgpJtbk/l9ijZf1osEqDz/kGI4WNQ5q/SlmVXnbQ== X-Received: by 2002:adf:e946:0:b0:337:bcdf:2dd5 with SMTP id m6-20020adfe946000000b00337bcdf2dd5mr61918wrn.16.1705542923066; Wed, 17 Jan 2024 17:55:23 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id d12-20020adfe88c000000b00337be3b02aasm2818016wrm.100.2024.01.17.17.55.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jan 2024 17:55:22 -0800 (PST) Message-ID: In-Reply-To: References: Date: Thu, 18 Jan 2024 01:55:18 +0000 Subject: [PATCH v2 4/4] submodule-config.c: strengthen URL fsck check Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Patrick Steinhardt , Jeff King , Victoria Dye , Victoria Dye From: Victoria Dye From: Victoria Dye Update the validation of "curl URL" submodule URLs (i.e. those that specify an "http[s]" or "ftp[s]" protocol) in 'check_submodule_url()' to catch more invalid URLs. The existing validation using 'credential_from_url_gently()' parses certain URLs incorrectly, leading to invalid submodule URLs passing 'git fsck' checks. Conversely, 'url_normalize()' - used to validate remote URLs in 'remote_get()' - correctly identifies the invalid URLs missed by 'credential_from_url_gently()'. To catch more invalid cases, replace 'credential_from_url_gently()' with 'url_normalize()' followed by a 'url_decode()' and a check for newlines (mirroring 'check_url_component()' in the 'credential_from_url_gently()' validation). Signed-off-by: Victoria Dye --- submodule-config.c | 16 +++++++++++----- t/t7450-bad-git-dotfiles.sh | 11 +---------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/submodule-config.c b/submodule-config.c index 3b295e9f89c..54130f6a385 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -15,7 +15,7 @@ #include "thread-utils.h" #include "tree-walk.h" #include "url.h" -#include "credential.h" +#include "urlmatch.h" /* * submodule cache lookup structure @@ -350,12 +350,18 @@ int check_submodule_url(const char *url) } else if (url_to_curl_url(url, &curl_url)) { - struct credential c = CREDENTIAL_INIT; int ret = 0; - if (credential_from_url_gently(&c, curl_url, 1) || - !*c.host) + char *normalized = url_normalize(curl_url, NULL); + if (normalized) { + char *decoded = url_decode(normalized); + if (strchr(decoded, '\n')) + ret = -1; + free(normalized); + free(decoded); + } else { ret = -1; - credential_clear(&c); + } + return ret; } diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index c73b1c92ecc..46d4fb0354b 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -63,6 +63,7 @@ test_expect_success 'check urls' ' ./%0ahost=example.com/foo.git https://one.example.com/evil?%0ahost=two.example.com https:///example.com/foo.git + http://example.com:test/foo.git https::example.com/foo.git http:::example.com/foo.git EOF @@ -70,16 +71,6 @@ test_expect_success 'check urls' ' test_cmp expect actual ' -# NEEDSWORK: the URL checked here is not valid (and will not work as a remote if -# a user attempts to clone it), but the fsck check passes. -test_expect_failure 'url check misses invalid cases' ' - test-tool submodule check-url >actual <<-\EOF && - http://example.com:test/foo.git - EOF - - test_must_be_empty actual -' - test_expect_success 'create innocent subrepo' ' git init innocent && git -C innocent commit --allow-empty -m foo