Skip to content

Commit

Permalink
Add unit test for img filtering
Browse files Browse the repository at this point in the history
Test case for a644733.
  • Loading branch information
emk committed Dec 20, 2008
1 parent 284f1ac commit ff0113a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
10 changes: 5 additions & 5 deletions RAILS-2.2-TODO.txt
Expand Up @@ -3,8 +3,8 @@ release based on Rails 2.2.

/ Try to upgrade to gem version of coderay
/ Fix TZInfo to work again
Make sure we know what's up with plugins
Security audit--see below
Make sure we know what's up with plugins
We need to review our TODO comments

== Security
Expand All @@ -16,10 +16,10 @@ X Can we restrict admin cookies to /admin ? No--need /accounts, too.
/ Make sure logging out clears all relevant cookies and tokens
/ Check for session fixation attacks
/ Make sure cookies are HTTP-only whenever possible
Cross-site scripting
/ Cross-site scripting
/ Turn on protect_against_forgery
Check all fields in comments
, It looks like the failed comment error form has issues
/ Check all fields in comments
/ It looks like the failed comment error form has issues
/ Fix comment submission form to not use mass assignment
/ Check macro:* bodies and parameters
/ Make sure comment output is properly sanitized in any case
Expand All @@ -32,7 +32,6 @@ X Can we restrict admin cookies to /admin ? No--need /accounts, too.
/ Don't ship :session_key in environment.rb!
/ Do we need to override verifiable_request_format? No.
/ Check redirection in lib/authenticated_system.rb
Detect mass assignment failures in unit tests
/ Review mass assignment in public controllers - comments
/ Check regexes for ^ and $
/ Filter IMG tags
Expand All @@ -52,6 +51,7 @@ X Can we restrict admin cookies to /admin ? No--need /accounts, too.
Require the user to enter the old password when changing it
This will break our password reset system, actually
Require password to change e-mail address?
Detect mass assignment failures in unit tests - not really security issue

== Mass assignment protection

Expand Down
9 changes: 9 additions & 0 deletions test/unit/comment_drop_test.rb
Expand Up @@ -52,6 +52,15 @@ def test_should_show_filtered_text
assert_equal '<p><strong>test</strong> comment</p>', liquid.before_method(:body)
end

def test_should_not_allow_img_tags
# img tags can be used in CSRF attacks against actions that
# accidentally allow GET requests to perform destructive actions.
comment = contents(:welcome_comment)
comment.body = 'a<img src="/admin/site/destroy/1" />b'
comment.save!
assert_equal '<p>ab</p>', comment.to_liquid.before_method(:body)
end

def test_comment_url
t = Time.now.utc - 3.days
assert_equal "/#{t.year}/#{t.month}/#{t.day}/welcome-to-mephisto", @comment.url
Expand Down

0 comments on commit ff0113a

Please sign in to comment.