File CVE-2023-29007-1.patch of Package git (Revision f6da5411e7ab33c4e7b7d220b4c706fc)
Currently displaying revision f6da5411e7ab33c4e7b7d220b4c706fc , Show latest
xxxxxxxxxx
1
commit a5bb10fd5e74101e7c07da93e7c32bbe60f6173a
2
Author: Taylor Blau <me@ttaylorr.com>
3
Date: Thu Apr 6 14:07:58 2023 -0400
4
5
config: avoid fixed-sized buffer when renaming/deleting a section
6
7
When renaming (or deleting) a section of configuration, Git uses the
8
function `git_config_copy_or_rename_section_in_file()` to rewrite the
9
configuration file after applying the rename or deletion to the given
10
section.
11
12
To do this, Git repeatedly calls `fgets()` to read the existing
13
configuration data into a fixed size buffer.
14
15
When the configuration value under `old_name` exceeds the size of the
16
buffer, we will call `fgets()` an additional time even if there is no
17
newline in the configuration file, since our read length is capped at
18
`sizeof(buf)`.
19
20
If the first character of the buffer (after zero or more characters
21
satisfying `isspace()`) is a '[', Git will incorrectly treat it as
22
beginning a new section when the original section is being removed. In
23
other words, a configuration value satisfying this criteria can
24
incorrectly be considered as a new secftion instead of a variable in the
25
original section.
26
27
Avoid this issue by using a variable-width buffer in the form of a
28
strbuf rather than a fixed-with region on the stack. A couple of small
29
points worth noting:
30
31
- Using a strbuf will cause us to allocate arbitrary sizes to match
32
the length of each line. In practice, we don't expect any
33
reasonable configuration files to have lines that long, and a
34
bandaid will be introduced in a later patch to ensure that this is
35
the case.
36
37
- We are using strbuf_getwholeline() here instead of strbuf_getline()
38
in order to match `fgets()`'s behavior of leaving the trailing LF
39
character on the buffer (as well as a trailing NUL).
40
41
This could be changed later, but using strbuf_getwholeline() changes
42
the least about this function's implementation, so it is picked as
43
the safest path.
44
45
- It is temping to want to replace the loop to skip over characters
46
matching isspace() at the beginning of the buffer with a convenience
47
function like `strbuf_ltrim()`. But this is the wrong approach for a
48
couple of reasons:
49
50
First, it involves a potentially large and expensive `memmove()`
51
which we would like to avoid. Second, and more importantly, we also
52
*do* want to preserve those spaces to avoid changing the output of
53
other sections.
54
55
In all, this patch is a minimal replacement of the fixed-width buffer in
56
`git_config_copy_or_rename_section_in_file()` to instead use a `struct
57
strbuf`.
58
59
Reported-by: André Baptista <andre@ethiack.com>
60
Reported-by: Vítor Pinho <vitor@ethiack.com>
61
Helped-by: Patrick Steinhardt <ps@pks.im>
62
Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
63
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
64
Signed-off-by: Taylor Blau <me@ttaylorr.com>
65
66
Index: git-2.26.2/config.c
67
===================================================================
68
--- git-2.26.2.orig/config.c 2020-04-20 15:52:30.000000000 +0000
69
+++ git-2.26.2/config.c 2023-04-24 13:25:57.672390778 +0000
70
71
char *filename_buf = NULL;
72
struct lock_file lock = LOCK_INIT;
73
int out_fd;
74
- char buf[1024];
75
+ struct strbuf buf = STRBUF_INIT;
76
FILE *config_file = NULL;
77
struct stat st;
78
struct strbuf copystr = STRBUF_INIT;
79
80
goto out;
81
}
82
83
- while (fgets(buf, sizeof(buf), config_file)) {
84
+ while (!strbuf_getwholeline(&buf, config_file, '\n')) {
85
int i;
86
int length;
87
int is_section = 0;
88
- char *output = buf;
89
- for (i = 0; buf[i] && isspace(buf[i]); i++)
90
+ char *output = buf.buf;
91
+ for (i = 0; buf.buf[i] && isspace(buf.buf[i]); i++)
92
; /* do nothing */
93
- if (buf[i] == '[') {
94
+ if (buf.buf[i] == '[') {
95
/* it's a section */
96
int offset;
97
is_section = 1;
98
99
strbuf_reset(©str);
100
}
101
102
- offset = section_name_match(&buf[i], old_name);
103
+ offset = section_name_match(&buf.buf[i], old_name);
104
if (offset > 0) {
105
ret++;
106
if (new_name == NULL) {
107
108
out_no_rollback:
109
free(filename_buf);
110
config_store_data_clear(&store);
111
+ strbuf_release(&buf);
112
return ret;
113
}
114
115
Index: git-2.26.2/t/t1300-config.sh
116
===================================================================
117
--- git-2.26.2.orig/t/t1300-config.sh 2023-04-24 13:25:44.275434669 +0000
118
+++ git-2.26.2/t/t1300-config.sh 2023-04-24 13:25:47.475424203 +0000
119
120
test_must_fail git config --rename-section branch.zwei "bogus name"
121
'
122
123
-test_expect_failure 'renaming a section with a long line' '
124
+test_expect_success 'renaming a section with a long line' '
125
{
126
printf "[b]\\n" &&
127
printf " c = d %1024s [a] e = f\\n" " " &&
128
129
test_must_fail git config -f y b.e
130
'
131
132
-test_expect_failure 'renaming an embedded section with a long line' '
133
+test_expect_success 'renaming an embedded section with a long line' '
134
{
135
printf "[b]\\n" &&
136
printf " c = d %1024s [a] [foo] e = f\\n" " " &&
137