Merge lp:~mhall119/summit/xss-vulnerability-fix into lp:~summit-hackers/summit/1.x

Proposed by Michael Hall
Status: Merged
Approved by: Nigel Babu
Approved revision: 144
Merged at revision: 144
Proposed branch: lp:~mhall119/summit/xss-vulnerability-fix
Merge into: lp:~summit-hackers/summit/1.x
Diff against target: 193 lines (+166/-2)
2 files modified
summit/sponsor/templates/sponsor/review.html (+1/-1)
summit/sponsor/tests.py (+165/-1)
To merge this branch: bzr merge lp:~mhall119/summit/xss-vulnerability-fix
Reviewer Review Type Date Requested Status
Nigel Babu (community) Approve
Review via email: mp+73091@code.launchpad.net

Description of the change

Overview
========
Sponsorship application data was being displayed unescaped, allowing javascript injection into the pages.

Details
=======
Django will escape data being inserted into a template by default, but due to a combination of template filters the sponsorship.about field was having it's unescaped data inserted, which allowed for someone to submit a sponsorship application containing embedded javascript that would be executed by any user viewing their application. This MP contains tests for specifically this behavior, and changes the order of filters being applied so that the final result will still be properly escaped.

To post a comment you must log in.
Revision history for this message
Nigel Babu (nigelbabu) wrote :

Ack. Good catch. I'm tempted to Rick Roll someone before we merge it in though :P

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'summit/sponsor/templates/sponsor/review.html'
--- summit/sponsor/templates/sponsor/review.html 2010-10-03 22:43:07 +0000
+++ summit/sponsor/templates/sponsor/review.html 2011-08-26 19:16:19 +0000
@@ -17,7 +17,7 @@
17</table>17</table>
1818
19<p>19<p>
20{{ sponsorship.about|escape|paras|urlize }}20{{ sponsorship.about|escape|urlize|paras }}
21</p>21</p>
2222
23<!--23<!--
2424
=== modified file 'summit/sponsor/tests.py'
--- summit/sponsor/tests.py 2011-08-22 16:12:36 +0000
+++ summit/sponsor/tests.py 2011-08-26 19:16:19 +0000
@@ -23,6 +23,8 @@
23from django.contrib.auth.models import User23from django.contrib.auth.models import User
24from summit.schedule.models import Summit24from summit.schedule.models import Summit
25from summit.sponsor.models import (25from summit.sponsor.models import (
26 Sponsorship,
27 SponsorshipScore,
26 SponsorshipSuggestion,28 SponsorshipSuggestion,
27 SponsorshipSuggestionScore,29 SponsorshipSuggestionScore,
28 NonLaunchpadSponsorship,30 NonLaunchpadSponsorship,
@@ -147,5 +149,167 @@
147 }149 }
148 form = SponsorshipSuggestionForm(form_data)150 form = SponsorshipSuggestionForm(form_data)
149 self.assertFalse(form.is_valid())151 self.assertFalse(form.is_valid())
150 152
153
154class SponsorshipReviewTestCase(djangotest.TestCase):
155
156 def setUp(self):
157 self.summit = Summit.objects.create(
158 name='test-summit',
159 title='Test Summit',
160 location='Test Location',
161 description='Test Summit Description',
162 timezone='UTC',
163 last_update=datetime.datetime.now(),
164 state='sponsor',
165 date_start=(datetime.datetime.now() + datetime.timedelta(days=1)),
166 date_end=(datetime.datetime.now() + datetime.timedelta(days=6)),
167 )
168
169 self.user = User.objects.create(
170 username='testuser',
171 first_name='Test',
172 last_name='User',
173 )
174
175 self.sponsorship = Sponsorship.objects.create(
176 user=self.user,
177 summit=self.summit,
178 location='Test Location',
179 country='Test Country',
180 about='Test About Description',
181 needs_travel=False,
182 needs_accomodation=False,
183 would_crew=True,
184 diet='Test Diet',
185 further_info='Test Further Info',
186 )
187
188 self.scorer = User.objects.create(
189 username='testscorer',
190 first_name='Test',
191 last_name='Scorer',
192 is_superuser=True,
193 )
194 self.scorer.set_password('password')
195 self.scorer.save()
196
197 self.score = SponsorshipScore.objects.create(
198 sponsorship=self.sponsorship,
199 user=self.scorer,
200 score=1,
201 comment='Test Comment',
202 )
203
204 def test_sponsorship_review_display(self):
205 self.client.login(username='testscorer', password='password')
206 response = self.client.get('/test-summit/sponsorship/review/%s' % self.sponsorship.id)
207 self.assertContains(response, '<h1>Test User</h1>', 1)
208 self.assertContains(response, '<th>Location:</th><td>Test Location</td>', 1)
209 self.assertContains(response, '<th>Country:</th><td>Test Country</td>', 1)
210 self.assertContains(response, '<p>\nTest About Description\n</p>', 1)
211
212 def test_sponsorship_about_paras_filtering(self):
213 self.sponsorship.about = 'Test\nAbout\nDescription'
214 self.sponsorship.save()
215
216 self.client.login(username='testscorer', password='password')
217 response = self.client.get('/test-summit/sponsorship/review/%s' % self.sponsorship.id)
218 self.assertContains(response, '<p>\nTest</p>\n<p>\nAbout</p>\n<p>\nDescription\n</p>', 1)
219
220 def test_sponsorship_about_urlize_filtering(self):
221 self.sponsorship.about = 'Test http://example.org Description'
222 self.sponsorship.save()
223
224 self.client.login(username='testscorer', password='password')
225 response = self.client.get('/test-summit/sponsorship/review/%s' % self.sponsorship.id)
226 self.assertContains(response, '<p>\nTest <a href="http://example.org" rel="nofollow">http://example.org</a> Description\n</p>', 1)
227
228 def test_sponsorship_xss_escaping(self):
229 self.sponsorship.location = '"><script>alert(/xss/)</script>'
230 self.sponsorship.country = '"><script>alert(/xss/)</script>'
231 self.sponsorship.about = '"><script>alert(/xss/)</script>'
232 self.sponsorship.save()
233
234 self.user.first_name= '"><script>alert(/xss/)</script>'
235 self.user.last_name=''
236 self.user.save()
237
238 self.client.login(username='testscorer', password='password')
239 response = self.client.get('/test-summit/sponsorship/review/%s' % self.sponsorship.id)
240 # All displayed fields should have their html escaped
241 self.assertContains(response, '<h1>&quot;&gt;&lt;script&gt;alert(/xss/)&lt;/script&gt;</h1>', 1)
242 self.assertContains(response, '<th>Location:</th><td>&quot;&gt;&lt;script&gt;alert(/xss/)&lt;/script&gt;</td>', 1)
243 self.assertContains(response, '<th>Country:</th><td>&quot;&gt;&lt;script&gt;alert(/xss/)&lt;/script&gt;</td>', 1)
244 self.assertContains(response, '"><script>alert(/xss/)</script>', 0)
245
246
247class SuggestionReviewTestCase(djangotest.TestCase):
248
249 def setUp(self):
250 self.summit = Summit.objects.create(
251 name='test-summit',
252 title='Test Summit',
253 location='Test Location',
254 description='Test Summit Description',
255 timezone='UTC',
256 last_update=datetime.datetime.now(),
257 state='sponsor',
258 date_start=(datetime.datetime.now() + datetime.timedelta(days=1)),
259 date_end=(datetime.datetime.now() + datetime.timedelta(days=6)),
260 )
261
262 self.user = User.objects.create(
263 username='testuser',
264 first_name='Test',
265 last_name='User',
266 )
267
268 self.sponsorship = SponsorshipSuggestion.objects.create(
269 suggested_by=self.user,
270 name='Test Suggestion',
271 launchpad_name='testsuggestion',
272 summit=self.summit,
273 location='Test Location',
274 country='Test Country',
275 about='Test About Description',
276 needs_travel=False,
277 needs_accomodation=False,
278 would_crew=True,
279 diet='Test Diet',
280 further_info='Test Further Info',
281 )
282
283 self.scorer = User.objects.create(
284 username='testscorer',
285 first_name='Test',
286 last_name='Scorer',
287 is_superuser=True,
288 )
289 self.scorer.set_password('password')
290 self.scorer.save()
291
292 self.score = SponsorshipSuggestionScore.objects.create(
293 sponsorship=self.sponsorship,
294 user=self.scorer,
295 score=1,
296 comment='Test Comment',
297 )
298
299 def test_sponsorshipsuggestion_review_display(self):
300 self.client.login(username='testscorer', password='password')
301 response = self.client.get('/test-summit/suggestedsponsorship/review/%s' % self.sponsorship.id)
302 self.assertContains(response, '<h1>Test Suggestion</h1>', 1)
303
304 def test_sponsorshipsuggestion_xss_escaping(self):
305 self.client.login(username='testscorer', password='password')
306 self.sponsorship.name = '"><script>alert(/xss/)</script>'
307 self.sponsorship.location = '"><script>alert(/xss/)</script>'
308 self.sponsorship.country = '"><script>alert(/xss/)</script>'
309 self.sponsorship.about = '"><script>alert(/xss/)</script>'
310 self.sponsorship.save()
311 response = self.client.get('/test-summit/suggestedsponsorship/review/%s' % self.sponsorship.id)
312 # All displayed fields should have their html escaped
313 self.assertContains(response, '"><script>alert(/xss/)</script>', 0)
314
151315

Subscribers

People subscribed via source and target branches

to all changes: