@matheusbsbs / matheus-bagliano / commits / c7620bd0f3

upload-pack: disable object filtering when disabled by config

When upload-pack gained partial clone support (v2.17.0-rc0~132^2~12, 2017-12-08), it was guarded by the uploadpack.allowFilter config item to allow server operators to control when they start supporting it. That config item didn't go far enough, though: it controls whether the 'filter' capability is advertised, but if a (custom) client ignores the capability advertisement and passes a filter specification anyway, the server would handle that despite allowFilter being false. This is particularly significant if a security bug is discovered in this new experimental partial clone code. Installations without uploadpack.allowFilter ought not to be affected since they don't intend to support partial clone, but they would be swept up into being vulnerable. Simplify and limit the attack surface by making uploadpack.allowFilter disable the feature, not just the advertisement of it. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Nieder committed Mar 28, 2018 at 13:33 UTC c7620bd0f35dddf8b8519da6fbf97014f46d0710

2 files changed +5 -5

Documentation/config.txt

+1 -1

@@ -3270,7 +3270,7 @@ uploadpack.packObjectsHook::

3270 stdout.

3271

3272 uploadpack.allowFilter::

3273 - If this option is set, `upload-pack` will advertise partial

3273 + If this option is set, `upload-pack` will support partial

3274 clone and partial fetch object filtering.

3275 +

3276 Note that this configuration variable is ignored if it is seen in the

upload-pack.c

+4 -4

@@ -68,7 +68,7 @@ static int stateless_rpc;

68 static const char *pack_objects_hook;

70 static int filter_capability_requested;

71 -static int filter_advertise;

71 +static int allow_filter;

72 static struct list_objects_filter_options filter_options;

74 static void reset_timeout(void)

@@ -845,7 +845,7 @@ static void receive_needs(void)

845 no_progress = 1;

846 if (parse_feature_request(features, "include-tag"))

847 use_include_tag = 1;

848 - if (parse_feature_request(features, "filter"))

848 + if (allow_filter && parse_feature_request(features, "filter"))

849 filter_capability_requested = 1;

850

851 o = parse_object(&oid_buf);

@@ -975,7 +975,7 @@ static int send_ref(const char *refname, const struct object_id *oid,

975 " allow-reachable-sha1-in-want" : "",

976 stateless_rpc ? " no-done" : "",

977 symref_info.buf,

978 - filter_advertise ? " filter" : "",

978 + allow_filter ? " filter" : "",

979 git_user_agent_sanitized());

980 strbuf_release(&symref_info);

981 } else {

@@ -1055,7 +1055,7 @@ static int upload_pack_config(const char *var, const char *value, void *unused)

1055 if (!strcmp("uploadpack.packobjectshook", var))

1056 return git_config_string(&pack_objects_hook, var, value);

1057 } else if (!strcmp("uploadpack.allowfilter", var)) {

1058 - filter_advertise = git_config_bool(var, value);

1058 + allow_filter = git_config_bool(var, value);

1059 }

1060 return parse_hide_refs_config(var, value, "uploadpack");

1061 }