Skip to content

Commit b0c1565

Browse files
flaixflaix
authored
Merge pull request #1411 from gitblit/1410-vulnerability-userdb into mixup-1.9
Fix vulnerability in config user service backend
2 parents 456813c + 9b4afad commit b0c1565

File tree

3 files changed

+382
-6
lines changed

3 files changed

+382
-6
lines changed

‎src/main/java/com/gitblit/StoredUserConfig.java

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,12 @@ public void save() throws IOException {
7373
}
7474

7575
private static void writeSection(PrintWriter printWriter, String key, Section section) {
76-
printWriter.printf("[%s \"%s\"]\n", section.getName(), section.getSubSection());
76+
if (section.getSubSection() == null) {
77+
printWriter.printf("[%s]\n", section.getName());
78+
}
79+
else {
80+
printWriter.printf("[%s \"%s\"]\n", section.getName(), section.getSubSection());
81+
}
7782
for (Entry entry : section.getEntries().values()) {
7883
writeEntry(printWriter, entry.getKey(), entry.getValue());
7984
}
@@ -84,13 +89,52 @@ private static void writeEntry(PrintWriter printWriter, String key, String value
8489
}
8590

8691
private static String escape(String value) {
87-
String fixedValue = '#' == value.charAt(0) ? "\"" + value + "\"" : value;
88-
fixedValue = fixedValue.replace("\\", "\\\\");
89-
return fixedValue;
92+
boolean quoteIt = false;
93+
StringBuilder fixedValue = new StringBuilder(value.length() + 20);
94+
95+
for (char c : value.toCharArray()) {
96+
switch (c) {
97+
case '\n':
98+
fixedValue.append("\\n");
99+
break;
100+
101+
case '\t':
102+
fixedValue.append("\\t");
103+
break;
104+
105+
case '\b':
106+
fixedValue.append("\\b");
107+
break;
108+
109+
case '\\':
110+
fixedValue.append("\\\\");
111+
break;
112+
113+
case '"':
114+
fixedValue.append("\\\"");
115+
break;
116+
117+
case ';':
118+
case '#':
119+
quoteIt = true;
120+
fixedValue.append(c);
121+
break;
122+
123+
default:
124+
fixedValue.append(c);
125+
break;
126+
}
127+
}
128+
129+
if (quoteIt) {
130+
fixedValue.insert(0,"\"");
131+
fixedValue.append("\"");
132+
}
133+
return fixedValue.toString();
90134
}
91135

92136
private static String generateKey(String key, String subKey) {
93-
return "k:" + key + "s:" + subKey;
137+
return "k:" + key + "s:" + (subKey == null ? "" : subKey);
94138
}
95139

96140
private static class Section {

‎src/test/java/com/gitblit/StoredUserConfigTest.java

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
package com.gitblit;
2+
3+
import org.eclipse.jgit.lib.StoredConfig;
4+
import org.eclipse.jgit.storage.file.FileBasedConfig;
5+
import org.eclipse.jgit.util.FS;
6+
import org.junit.After;
7+
import org.junit.Before;
8+
import org.junit.Test;
9+
10+
import java.io.File;
11+
12+
import static org.junit.Assert.*;
13+
14+
public class StoredUserConfigTest
15+
{
16+
private static File file;
17+
18+
@Before
19+
public void setup()
20+
{
21+
file = new File("./suc-test.conf");
22+
file.delete();
23+
}
24+
25+
@After
26+
public void teardown()
27+
{
28+
file.delete();
29+
}
30+
31+
32+
33+
@Test
34+
public void testSection() throws Exception
35+
{
36+
StoredUserConfig config = new StoredUserConfig(file);
37+
config.setString("USER", "norman", "key", "value");
38+
config.setString("USER", "admin", "displayName", "marusha");
39+
config.setString("USER", null, "role", "none");
40+
41+
config.setString("TEAM", "admin", "role", "admin");
42+
config.setString("TEAM", "ci", "email", "[email protected]");
43+
config.setString("TEAM", null, "displayName", "noone");
44+
45+
config.save();
46+
47+
StoredConfig cfg = new FileBasedConfig(file, FS.detect());
48+
cfg.load();
49+
assertEquals("value", cfg.getString("USER", "norman", "key"));
50+
assertEquals("marusha", cfg.getString("USER", "admin", "displayName"));
51+
assertEquals("none", cfg.getString("USER", null, "role"));
52+
53+
assertEquals("admin", cfg.getString("TEAM", "admin", "role"));
54+
assertEquals("[email protected]", cfg.getString("TEAM", "ci", "email"));
55+
assertEquals("noone", cfg.getString("TEAM", null, "displayName"));
56+
}
57+
58+
59+
@Test
60+
public void testStringFields() throws Exception
61+
{
62+
StoredUserConfig config = new StoredUserConfig(file);
63+
config.setString("USER", "admin", "password", "secret");
64+
config.setString("USER", "admin", "displayName", "marusha");
65+
config.setString("USER", "admin", "email", "[email protected]");
66+
67+
config.setString("USER", "other", "password", "password");
68+
config.setString("USER", "other", "displayName", "mama");
69+
config.setString("USER", "other", "email", "[email protected]");
70+
config.setString("USER", "other", "repository", "RW+:repo1");
71+
config.setString("USER", "other", "repository", "RW+:repo2");
72+
73+
config.setString("USER", null, "displayName", "default");
74+
75+
config.save();
76+
77+
StoredConfig cfg = new FileBasedConfig(file, FS.detect());
78+
cfg.load();
79+
assertEquals("secret", cfg.getString("USER", "admin", "password"));
80+
assertEquals("marusha", cfg.getString("USER", "admin", "displayName"));
81+
assertEquals("[email protected]", cfg.getString("USER", "admin", "email"));
82+
83+
assertEquals("password", cfg.getString("USER", "other", "password"));
84+
assertEquals("mama", cfg.getString("USER", "other", "displayName"));
85+
assertEquals("[email protected]", cfg.getString("USER", "other", "email"));
86+
87+
String[] stringList = cfg.getStringList("USER", "other", "repository");
88+
assertNotNull(stringList);
89+
assertEquals(2, stringList.length);
90+
int i = 0;
91+
for (String s : stringList) {
92+
if (s.equalsIgnoreCase("RW+:repo1")) i += 1;
93+
else if (s.equalsIgnoreCase("RW+:repo2")) i += 2;
94+
}
95+
assertEquals("Not all repository strings found", 3, i);
96+
97+
assertEquals("default", cfg.getString("USER", null, "displayName"));
98+
}
99+
100+
101+
@Test
102+
public void testBooleanFields() throws Exception
103+
{
104+
StoredUserConfig config = new StoredUserConfig(file);
105+
config.setBoolean("USER", "admin", "emailMeOnMyTicketChanges", true);
106+
config.setBoolean("USER", "master", "emailMeOnMyTicketChanges", false);
107+
config.setBoolean("TEAM", "admin", "excludeFromFederation", true);
108+
config.setBoolean("USER", null, "excludeFromFederation", false);
109+
110+
config.save();
111+
112+
StoredConfig cfg = new FileBasedConfig(file, FS.detect());
113+
cfg.load();
114+
assertTrue(cfg.getBoolean("USER", "admin", "emailMeOnMyTicketChanges", false));
115+
assertFalse(cfg.getBoolean("USER", "master", "emailMeOnMyTicketChanges", true));
116+
assertTrue(cfg.getBoolean("TEAM", "admin", "excludeFromFederation", false));
117+
assertFalse(cfg.getBoolean("USER", null, "excludeFromFederation", true));
118+
}
119+
120+
121+
@Test
122+
public void testHashEscape() throws Exception
123+
{
124+
StoredUserConfig config = new StoredUserConfig(file);
125+
config.setString("USER", "admin", "role", "#admin");
126+
127+
config.setString("USER", "other", "role", "#none");
128+
config.setString("USER", "other", "displayName", "big#");
129+
config.setString("USER", "other", "email", "user#[email protected]");
130+
131+
config.save();
132+
133+
StoredConfig cfg = new FileBasedConfig(file, FS.detect());
134+
cfg.load();
135+
assertEquals("#admin", cfg.getString("USER", "admin", "role"));
136+
assertEquals("#none", cfg.getString("USER", "other", "role"));
137+
assertEquals("big#", cfg.getString("USER", "other", "displayName"));
138+
assertEquals("user#[email protected]", cfg.getString("USER", "other", "email"));
139+
}
140+
141+
142+
@Test
143+
public void testCtrlEscape() throws Exception
144+
{
145+
StoredUserConfig config = new StoredUserConfig(file);
146+
config.setString("USER", "name", "password", "bing\bbong");
147+
config.setString("USER", "name", "role", "domain\\admin");
148+
config.setString("USER", "name", "displayName", "horny\n\telephant");
149+
config.setString("USER", "name", "org", "\tbig\tblue");
150+
config.setString("USER", "name", "unit", "the end\n");
151+
152+
config.setString("USER", null, "unit", "the\ndefault");
153+
154+
config.save();
155+
156+
StoredConfig cfg = new FileBasedConfig(file, FS.detect());
157+
cfg.load();
158+
assertEquals("bing\bbong", cfg.getString("USER", "name", "password"));
159+
assertEquals("domain\\admin", cfg.getString("USER", "name", "role"));
160+
assertEquals("horny\n\telephant", cfg.getString("USER", "name", "displayName"));
161+
assertEquals("\tbig\tblue", cfg.getString("USER", "name", "org"));
162+
assertEquals("the end\n", cfg.getString("USER", "name", "unit"));
163+
164+
assertEquals("the\ndefault", cfg.getString("USER", null, "unit"));
165+
}
166+
167+
168+
@Test
169+
public void testQuoteEscape() throws Exception
170+
{
171+
StoredUserConfig config = new StoredUserConfig(file);
172+
config.setString("USER", "dude", "password", "going\"places");
173+
config.setString("USER", "dude", "role", "\"dude\"");
174+
config.setString("USER", "dude", "displayName", "John \"The Dude\" Lebowski");
175+
config.setString("USER", "dude", "repo", "\"front matter");
176+
config.setString("USER", "dude", "peepo", "leadout\"");
177+
178+
config.save();
179+
180+
StoredConfig cfg = new FileBasedConfig(file, FS.detect());
181+
cfg.load();
182+
assertEquals("going\"places", cfg.getString("USER", "dude", "password"));
183+
assertEquals("\"dude\"", cfg.getString("USER", "dude", "role"));
184+
assertEquals("John \"The Dude\" Lebowski", cfg.getString("USER", "dude", "displayName"));
185+
assertEquals("\"front matter", cfg.getString("USER", "dude", "repo"));
186+
assertEquals("leadout\"", cfg.getString("USER", "dude", "peepo"));
187+
}
188+
189+
190+
@Test
191+
public void testUTF8() throws Exception
192+
{
193+
StoredUserConfig config = new StoredUserConfig(file);
194+
config.setString("USER", "ming", "password", "一\t二\n三");
195+
config.setString("USER", "ming", "displayName", "白老鼠");
196+
config.setString("USER", "ming", "peepo", "Mickey \"白老鼠\" Whitfield");
197+
198+
config.save();
199+
200+
StoredConfig cfg = new FileBasedConfig(file, FS.detect());
201+
cfg.load();
202+
assertEquals("一\t二\n三", cfg.getString("USER", "ming", "password"));
203+
assertEquals("白老鼠", cfg.getString("USER", "ming", "displayName"));
204+
assertEquals("Mickey \"白老鼠\" Whitfield", cfg.getString("USER", "ming", "peepo"));
205+
}
206+
207+
}

0 commit comments

Comments
 (0)