Mercurial > p > roundup > code
comparison test/test_cgi.py @ 5161:12190efa30d4
I realized that the __came_from and __redirect_to url parameters I
added to handle issues with the LoginAction and NewItemAction could
be used for XSS or other purposes.
So I check them using a new clean_url(url) function. This tries to
validate that the url is under the tracker's base url and that the
components of the url are properly url encoded. If it thinks something
is wrong with the url, it will raise a ValueError. I decided to not
attempt to fix the url's if there is an issue, better to bring it to the
tracker admin's attention.
Changed the code paths in NewItemAction and LoginAction that deal with
the form parameters to use the clean_url function on the form input
first.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Sat, 23 Jul 2016 14:00:49 -0400 |
| parents | f8a32b7331f1 |
| children | 3ee79a2d95d4 |
comparison
equal
deleted
inserted
replaced
| 5160:f8a32b7331f1 | 5161:12190efa30d4 |
|---|---|
| 1171 result = self.client.renderContext() | 1171 result = self.client.renderContext() |
| 1172 self.assertNotEqual(-1, result.index('<title>User listing - Roundup issue tracker</title>')) | 1172 self.assertNotEqual(-1, result.index('<title>User listing - Roundup issue tracker</title>')) |
| 1173 self.assertNotEqual(-1, result.index('ok message')) | 1173 self.assertNotEqual(-1, result.index('ok message')) |
| 1174 # print result | 1174 # print result |
| 1175 | 1175 |
| 1176 def testclean_url(self): | |
| 1177 ''' test the clean_url function ''' | |
| 1178 action = actions.Action(self.client) | |
| 1179 clean_url = action.clean_url | |
| 1180 | |
| 1181 # Christmas tree url: test of every component that passes | |
| 1182 self.assertEqual( | |
| 1183 clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1184 'http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue') | |
| 1185 | |
| 1186 # allow replacing http with https if base is http | |
| 1187 self.assertEqual( | |
| 1188 clean_url("https://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1189 'https://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue') | |
| 1190 | |
| 1191 | |
| 1192 # change base to use https and make sure we don't redirect to http | |
| 1193 saved_base = action.base | |
| 1194 action.base = "https://tracker.example/cgi-bin/roundup.cgi/bugs/" | |
| 1195 with self.assertRaises(ValueError) as cm: | |
| 1196 clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue") | |
| 1197 self.assertEqual(cm.exception.message, 'Base url https://tracker.example/cgi-bin/roundup.cgi/bugs/ requires https. Redirect url http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue uses http.') | |
| 1198 action.base = saved_base | |
| 1199 | |
| 1200 # url doesn't have to be valid to roundup, just has to be contained | |
| 1201 # inside of roundup. No zoik class is defined | |
| 1202 self.assertEqual(clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/zoik7;parm=bar?@template=foo&parm=(zot)#issue"), "http://tracker.example/cgi-bin/roundup.cgi/bugs/zoik7;parm=bar?@template=foo&parm=(zot)#issue") | |
| 1203 | |
| 1204 # test with wonky schemes | |
| 1205 with self.assertRaises(ValueError) as cm: | |
| 1206 clean_url("email://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1207 self.assertEqual(cm.exception.message, 'Unrecognized scheme in email://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue') | |
| 1208 | |
| 1209 with self.assertRaises(ValueError) as cm: | |
| 1210 clean_url("http%3a//tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1211 self.assertEqual(cm.exception.message, 'Unrecognized scheme in http%3a//tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue') | |
| 1212 | |
| 1213 # test different netloc/path prefix | |
| 1214 # assert port | |
| 1215 with self.assertRaises(ValueError) as cm: | |
| 1216 clean_url("http://tracker.example:1025/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1217 self.assertEqual(cm.exception.message, 'Net location in http://tracker.example:1025/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example') | |
| 1218 | |
| 1219 #assert user | |
| 1220 with self.assertRaises(ValueError) as cm: | |
| 1221 clean_url("http://user@tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1222 self.assertEqual(cm.exception.message, 'Net location in http://user@tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example') | |
| 1223 | |
| 1224 #assert user:password | |
| 1225 with self.assertRaises(ValueError) as cm: | |
| 1226 clean_url("http://user:pass@tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1227 self.assertEqual(cm.exception.message, 'Net location in http://user:pass@tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example') | |
| 1228 | |
| 1229 # try localhost http scheme | |
| 1230 with self.assertRaises(ValueError) as cm: | |
| 1231 clean_url("http://localhost/cgi-bin/roundup.cgi/bugs/user3") | |
| 1232 self.assertEqual(cm.exception.message, 'Net location in http://localhost/cgi-bin/roundup.cgi/bugs/user3 does not match base: tracker.example') | |
| 1233 | |
| 1234 # try localhost https scheme | |
| 1235 with self.assertRaises(ValueError) as cm: | |
| 1236 clean_url("https://localhost/cgi-bin/roundup.cgi/bugs/user3") | |
| 1237 self.assertEqual(cm.exception.message, 'Net location in https://localhost/cgi-bin/roundup.cgi/bugs/user3 does not match base: tracker.example') | |
| 1238 | |
| 1239 # try different host | |
| 1240 with self.assertRaises(ValueError) as cm: | |
| 1241 clean_url("http://bad.guys.are.us/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1242 self.assertEqual(cm.exception.message, 'Net location in http://bad.guys.are.us/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example') | |
| 1243 | |
| 1244 # change the base path to .../bug from .../bugs | |
| 1245 with self.assertRaises(ValueError) as cm: | |
| 1246 clean_url("http://tracker.example/cgi-bin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1247 self.assertEqual(cm.exception.message, 'Base path /cgi-bin/roundup.cgi/bugs/ is not a prefix for url http://tracker.example/cgi-bin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue') | |
| 1248 | |
| 1249 # change the base path eliminate - in cgi-bin | |
| 1250 with self.assertRaises(ValueError) as cm: | |
| 1251 clean_url("http://tracker.example/cgibin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1252 self.assertEqual(cm.exception.message, 'Base path /cgi-bin/roundup.cgi/bugs/ is not a prefix for url http://tracker.example/cgibin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue') | |
| 1253 | |
| 1254 | |
| 1255 # scan for unencoded characters | |
| 1256 # we skip schema and net location since unencoded character | |
| 1257 # are allowed only by an explicit match to a reference. | |
| 1258 # | |
| 1259 # break components with unescaped character '<' | |
| 1260 # path component | |
| 1261 with self.assertRaises(ValueError) as cm: | |
| 1262 clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/<user3;parm=bar?@template=foo&parm=(zot)#issue"), | |
| 1263 self.assertEqual(cm.exception.message, 'Path component (/cgi-bin/roundup.cgi/bugs/<user3) in http://tracker.example/cgi-bin/roundup.cgi/bugs/<user3;parm=bar?@template=foo&parm=(zot)#issue is not properly escaped') | |
| 1264 | |
| 1265 # params component | |
| 1266 with self.assertRaises(ValueError) as cm: | |
| 1267 clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=b<ar?@template=foo&parm=(zot)#issue"), | |
| 1268 self.assertEqual(cm.exception.message, 'Params component (parm=b<ar) in http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=b<ar?@template=foo&parm=(zot)#issue is not properly escaped') | |
| 1269 | |
| 1270 # query component | |
| 1271 with self.assertRaises(ValueError) as cm: | |
| 1272 clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=<foo>&parm=(zot)#issue"), | |
| 1273 self.assertEqual(cm.exception.message, 'Query component (@template=<foo>&parm=(zot)) in http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=<foo>&parm=(zot)#issue is not properly escaped') | |
| 1274 | |
| 1275 # fragment component | |
| 1276 with self.assertRaises(ValueError) as cm: | |
| 1277 clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#iss<ue"), | |
| 1278 self.assertEqual(cm.exception.message, 'Fragment component (iss<ue) in http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#iss<ue is not properly escaped') | |
| 1279 | |
| 1176 class TemplateTestCase(unittest.TestCase): | 1280 class TemplateTestCase(unittest.TestCase): |
| 1177 ''' Test the template resolving code, i.e. what can be given to @template | 1281 ''' Test the template resolving code, i.e. what can be given to @template |
| 1178 ''' | 1282 ''' |
| 1179 def setUp(self): | 1283 def setUp(self): |
| 1180 self.dirname = '_test_template' | 1284 self.dirname = '_test_template' |