diff options
Diffstat (limited to 'meta/recipes-devtools')
-rw-r--r-- | meta/recipes-devtools/git/files/CVE-2018-11235.patch | 288 | ||||
-rw-r--r-- | meta/recipes-devtools/git/git.inc | 3 |
2 files changed, 290 insertions, 1 deletions
diff --git a/meta/recipes-devtools/git/files/CVE-2018-11235.patch b/meta/recipes-devtools/git/files/CVE-2018-11235.patch new file mode 100644 index 0000000000..c272eac8d3 --- /dev/null +++ b/meta/recipes-devtools/git/files/CVE-2018-11235.patch | |||
@@ -0,0 +1,288 @@ | |||
1 | From 0383bbb9015898cbc79abd7b64316484d7713b44 Mon Sep 17 00:00:00 2001 | ||
2 | From: Jeff King <peff@peff.net> | ||
3 | Date: Mon, 30 Apr 2018 03:25:25 -0400 | ||
4 | Subject: [PATCH] submodule-config: verify submodule names as paths | ||
5 | |||
6 | Submodule "names" come from the untrusted .gitmodules file, | ||
7 | but we blindly append them to $GIT_DIR/modules to create our | ||
8 | on-disk repo paths. This means you can do bad things by | ||
9 | putting "../" into the name (among other things). | ||
10 | |||
11 | Let's sanity-check these names to avoid building a path that | ||
12 | can be exploited. There are two main decisions: | ||
13 | |||
14 | 1. What should the allowed syntax be? | ||
15 | |||
16 | It's tempting to reuse verify_path(), since submodule | ||
17 | names typically come from in-repo paths. But there are | ||
18 | two reasons not to: | ||
19 | |||
20 | a. It's technically more strict than what we need, as | ||
21 | we really care only about breaking out of the | ||
22 | $GIT_DIR/modules/ hierarchy. E.g., having a | ||
23 | submodule named "foo/.git" isn't actually | ||
24 | dangerous, and it's possible that somebody has | ||
25 | manually given such a funny name. | ||
26 | |||
27 | b. Since we'll eventually use this checking logic in | ||
28 | fsck to prevent downstream repositories, it should | ||
29 | be consistent across platforms. Because | ||
30 | verify_path() relies on is_dir_sep(), it wouldn't | ||
31 | block "foo\..\bar" on a non-Windows machine. | ||
32 | |||
33 | 2. Where should we enforce it? These days most of the | ||
34 | .gitmodules reads go through submodule-config.c, so | ||
35 | I've put it there in the reading step. That should | ||
36 | cover all of the C code. | ||
37 | |||
38 | We also construct the name for "git submodule add" | ||
39 | inside the git-submodule.sh script. This is probably | ||
40 | not a big deal for security since the name is coming | ||
41 | from the user anyway, but it would be polite to remind | ||
42 | them if the name they pick is invalid (and we need to | ||
43 | expose the name-checker to the shell anyway for our | ||
44 | test scripts). | ||
45 | |||
46 | This patch issues a warning when reading .gitmodules | ||
47 | and just ignores the related config entry completely. | ||
48 | This will generally end up producing a sensible error, | ||
49 | as it works the same as a .gitmodules file which is | ||
50 | missing a submodule entry (so "submodule update" will | ||
51 | barf, but "git clone --recurse-submodules" will print | ||
52 | an error but not abort the clone. | ||
53 | |||
54 | There is one minor oddity, which is that we print the | ||
55 | warning once per malformed config key (since that's how | ||
56 | the config subsystem gives us the entries). So in the | ||
57 | new test, for example, the user would see three | ||
58 | warnings. That's OK, since the intent is that this case | ||
59 | should never come up outside of malicious repositories | ||
60 | (and then it might even benefit the user to see the | ||
61 | message multiple times). | ||
62 | |||
63 | Credit for finding this vulnerability and the proof of | ||
64 | concept from which the test script was adapted goes to | ||
65 | Etienne Stalmans. | ||
66 | |||
67 | CVE: CVE-2018-11235 | ||
68 | Upstream-Status: Backport [https://github.com/gitster/git/commit/0383bbb9015898cbc79abd7b64316484d7713b44#diff-1772b951776d1647ca31a2256f7fe88f] | ||
69 | |||
70 | Signed-off-by: Jeff King <peff@peff.net> | ||
71 | Signed-off-by: Jagadeesh Krishnanjanappa <jkrishnanjanappa@mvista.com> | ||
72 | --- | ||
73 | builtin/submodule--helper.c | 24 ++++++++++++++ | ||
74 | git-submodule.sh | 5 +++ | ||
75 | submodule-config.c | 31 ++++++++++++++++++ | ||
76 | submodule-config.h | 7 +++++ | ||
77 | t/t7415-submodule-names.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++ | ||
78 | 5 files changed, 143 insertions(+) | ||
79 | create mode 100755 t/t7415-submodule-names.sh | ||
80 | |||
81 | diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c | ||
82 | index cbb17a902..b4b4d29d8 100644 | ||
83 | --- a/builtin/submodule--helper.c | ||
84 | +++ b/builtin/submodule--helper.c | ||
85 | @@ -1480,6 +1480,29 @@ static int is_active(int argc, const cha | ||
86 | return !is_submodule_active(the_repository, argv[1]); | ||
87 | } | ||
88 | |||
89 | +/* | ||
90 | + * Exit non-zero if any of the submodule names given on the command line is | ||
91 | + * invalid. If no names are given, filter stdin to print only valid names | ||
92 | + * (which is primarily intended for testing). | ||
93 | + */ | ||
94 | +static int check_name(int argc, const char **argv, const char *prefix) | ||
95 | +{ | ||
96 | + if (argc > 1) { | ||
97 | + while (*++argv) { | ||
98 | + if (check_submodule_name(*argv) < 0) | ||
99 | + return 1; | ||
100 | + } | ||
101 | + } else { | ||
102 | + struct strbuf buf = STRBUF_INIT; | ||
103 | + while (strbuf_getline(&buf, stdin) != EOF) { | ||
104 | + if (!check_submodule_name(buf.buf)) | ||
105 | + printf("%s\n", buf.buf); | ||
106 | + } | ||
107 | + strbuf_release(&buf); | ||
108 | + } | ||
109 | + return 0; | ||
110 | +} | ||
111 | + | ||
112 | #define SUPPORT_SUPER_PREFIX (1<<0) | ||
113 | |||
114 | struct cmd_struct { | ||
115 | @@ -1502,6 +1525,7 @@ static struct cmd_struct commands[] = { | ||
116 | {"push-check", push_check, 0}, | ||
117 | {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX}, | ||
118 | {"is-active", is_active, 0}, | ||
119 | + {"check-name", check_name, 0}, | ||
120 | }; | ||
121 | |||
122 | int cmd_submodule__helper(int argc, const char **argv, const char *prefix) | ||
123 | diff --git a/git-submodule.sh b/git-submodule.sh | ||
124 | index c0d0e9a4c..92750b9e2 100755 | ||
125 | --- a/git-submodule.sh | ||
126 | +++ b/git-submodule.sh | ||
127 | @@ -229,6 +229,11 @@ Use -f if you really want to add it." >& | ||
128 | sm_name="$sm_path" | ||
129 | fi | ||
130 | |||
131 | + if ! git submodule--helper check-name "$sm_name" | ||
132 | + then | ||
133 | + die "$(eval_gettext "'$sm_name' is not a valid submodule name")" | ||
134 | + fi | ||
135 | + | ||
136 | # perhaps the path exists and is already a git repo, else clone it | ||
137 | if test -e "$sm_path" | ||
138 | then | ||
139 | diff --git a/submodule-config.c b/submodule-config.c | ||
140 | index 4f58491dd..de54351c6 100644 | ||
141 | --- a/submodule-config.c | ||
142 | +++ b/submodule-config.c | ||
143 | @@ -190,6 +190,31 @@ static struct submodule *cache_lookup_na | ||
144 | return NULL; | ||
145 | } | ||
146 | |||
147 | +int check_submodule_name(const char *name) | ||
148 | +{ | ||
149 | + /* Disallow empty names */ | ||
150 | + if (!*name) | ||
151 | + return -1; | ||
152 | + | ||
153 | + /* | ||
154 | + * Look for '..' as a path component. Check both '/' and '\\' as | ||
155 | + * separators rather than is_dir_sep(), because we want the name rules | ||
156 | + * to be consistent across platforms. | ||
157 | + */ | ||
158 | + goto in_component; /* always start inside component */ | ||
159 | + while (*name) { | ||
160 | + char c = *name++; | ||
161 | + if (c == '/' || c == '\\') { | ||
162 | +in_component: | ||
163 | + if (name[0] == '.' && name[1] == '.' && | ||
164 | + (!name[2] || name[2] == '/' || name[2] == '\\')) | ||
165 | + return -1; | ||
166 | + } | ||
167 | + } | ||
168 | + | ||
169 | + return 0; | ||
170 | +} | ||
171 | + | ||
172 | static int name_and_item_from_var(const char *var, struct strbuf *name, | ||
173 | struct strbuf *item) | ||
174 | { | ||
175 | @@ -201,6 +226,12 @@ static int name_and_item_from_var(const | ||
176 | return 0; | ||
177 | |||
178 | strbuf_add(name, subsection, subsection_len); | ||
179 | + if (check_submodule_name(name->buf) < 0) { | ||
180 | + warning(_("ignoring suspicious submodule name: %s"), name->buf); | ||
181 | + strbuf_release(name); | ||
182 | + return 0; | ||
183 | + } | ||
184 | + | ||
185 | strbuf_addstr(item, key); | ||
186 | |||
187 | return 1; | ||
188 | diff --git a/submodule-config.h b/submodule-config.h | ||
189 | index d434ecdb4..103cc79dd 100644 | ||
190 | --- a/submodule-config.h | ||
191 | +++ b/submodule-config.h | ||
192 | @@ -48,4 +48,11 @@ extern const struct submodule *submodule | ||
193 | const char *key); | ||
194 | extern void submodule_free(void); | ||
195 | |||
196 | +/* | ||
197 | + * Returns 0 if the name is syntactically acceptable as a submodule "name" | ||
198 | + * (e.g., that may be found in the subsection of a .gitmodules file) and -1 | ||
199 | + * otherwise. | ||
200 | + */ | ||
201 | +int check_submodule_name(const char *name); | ||
202 | + | ||
203 | #endif /* SUBMODULE_CONFIG_H */ | ||
204 | diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh | ||
205 | new file mode 100755 | ||
206 | index 000000000..75fa071c6 | ||
207 | --- /dev/null | ||
208 | +++ b/t/t7415-submodule-names.sh | ||
209 | @@ -0,0 +1,76 @@ | ||
210 | +#!/bin/sh | ||
211 | + | ||
212 | +test_description='check handling of .. in submodule names | ||
213 | + | ||
214 | +Exercise the name-checking function on a variety of names, and then give a | ||
215 | +real-world setup that confirms we catch this in practice. | ||
216 | +' | ||
217 | +. ./test-lib.sh | ||
218 | + | ||
219 | +test_expect_success 'check names' ' | ||
220 | + cat >expect <<-\EOF && | ||
221 | + valid | ||
222 | + valid/with/paths | ||
223 | + EOF | ||
224 | + | ||
225 | + git submodule--helper check-name >actual <<-\EOF && | ||
226 | + valid | ||
227 | + valid/with/paths | ||
228 | + | ||
229 | + ../foo | ||
230 | + /../foo | ||
231 | + ..\foo | ||
232 | + \..\foo | ||
233 | + foo/.. | ||
234 | + foo/../ | ||
235 | + foo\.. | ||
236 | + foo\..\ | ||
237 | + foo/../bar | ||
238 | + EOF | ||
239 | + | ||
240 | + test_cmp expect actual | ||
241 | +' | ||
242 | + | ||
243 | +test_expect_success 'create innocent subrepo' ' | ||
244 | + git init innocent && | ||
245 | + git -C innocent commit --allow-empty -m foo | ||
246 | +' | ||
247 | + | ||
248 | +test_expect_success 'submodule add refuses invalid names' ' | ||
249 | + test_must_fail \ | ||
250 | + git submodule add --name ../../modules/evil "$PWD/innocent" evil | ||
251 | +' | ||
252 | + | ||
253 | +test_expect_success 'add evil submodule' ' | ||
254 | + git submodule add "$PWD/innocent" evil && | ||
255 | + | ||
256 | + mkdir modules && | ||
257 | + cp -r .git/modules/evil modules && | ||
258 | + write_script modules/evil/hooks/post-checkout <<-\EOF && | ||
259 | + echo >&2 "RUNNING POST CHECKOUT" | ||
260 | + EOF | ||
261 | + | ||
262 | + git config -f .gitmodules submodule.evil.update checkout && | ||
263 | + git config -f .gitmodules --rename-section \ | ||
264 | + submodule.evil submodule.../../modules/evil && | ||
265 | + git add modules && | ||
266 | + git commit -am evil | ||
267 | +' | ||
268 | + | ||
269 | +# This step seems like it shouldn't be necessary, since the payload is | ||
270 | +# contained entirely in the evil submodule. But due to the vagaries of the | ||
271 | +# submodule code, checking out the evil module will fail unless ".git/modules" | ||
272 | +# exists. Adding another submodule (with a name that sorts before "evil") is an | ||
273 | +# easy way to make sure this is the case in the victim clone. | ||
274 | +test_expect_success 'add other submodule' ' | ||
275 | + git submodule add "$PWD/innocent" another-module && | ||
276 | + git add another-module && | ||
277 | + git commit -am another | ||
278 | +' | ||
279 | + | ||
280 | +test_expect_success 'clone evil superproject' ' | ||
281 | + git clone --recurse-submodules . victim >output 2>&1 && | ||
282 | + ! grep "RUNNING POST CHECKOUT" output | ||
283 | +' | ||
284 | + | ||
285 | +test_done | ||
286 | -- | ||
287 | 2.13.3 | ||
288 | |||
diff --git a/meta/recipes-devtools/git/git.inc b/meta/recipes-devtools/git/git.inc index dd9d792c5c..bea23ec783 100644 --- a/meta/recipes-devtools/git/git.inc +++ b/meta/recipes-devtools/git/git.inc | |||
@@ -7,7 +7,8 @@ DEPENDS = "openssl curl zlib expat" | |||
7 | PROVIDES_append_class-native = " git-replacement-native" | 7 | PROVIDES_append_class-native = " git-replacement-native" |
8 | 8 | ||
9 | SRC_URI = "${KERNELORG_MIRROR}/software/scm/git/git-${PV}.tar.gz;name=tarball \ | 9 | SRC_URI = "${KERNELORG_MIRROR}/software/scm/git/git-${PV}.tar.gz;name=tarball \ |
10 | ${KERNELORG_MIRROR}/software/scm/git/git-manpages-${PV}.tar.gz;name=manpages" | 10 | ${KERNELORG_MIRROR}/software/scm/git/git-manpages-${PV}.tar.gz;name=manpages \ |
11 | file://CVE-2018-11235.patch" | ||
11 | 12 | ||
12 | S = "${WORKDIR}/git-${PV}" | 13 | S = "${WORKDIR}/git-${PV}" |
13 | 14 | ||