Skip to content
Commits on Dec 20, 2008
  1. add machinist to the tests

    committed Dec 20, 2008
  2. add Event blueprint

    committed Dec 20, 2008
  3. @emk

    Security: Finish first stage of audit

    What we've done: We've tried to protect against attacks by the "public".
    Most of our attention has been directed towards XSS, CSRF and other
    attacks by users who aren't logged in.
    
    Our security audit was based on the following principles:
    
     1) Users with access to /admin are (unfortunately) fully trusted.
        There are simply too many ways for them to escalate their privileges
        right now, if they're willing to use XSS and other attacks.
     2) Things which look "suspicious" were simply fixed, without any
        attempt to determine whether they could be exploited in the wild.
     3) Whenever possible, we instituted broad, automatic protections
        against entire classes of attacks.  These include SafeERB and
        read-only GET requests.  This means that we don't need to audit
        every single view, controller and plugin for subtle errors.
    
    What still needs work: My hacked version of SafeERB is currently
    breaking script/generate.
    emk committed Dec 20, 2008
  4. @emk

    Don't log "remember me" token

    This token can be trivially recovered from the database, so excluding it
    from the logs doesn't actually accomplish anything.  But there's no
    reason to include it in the logs, either.
    emk committed Dec 20, 2008
  5. @emk

    Security: Attempt to block auth of nil tokens, etc.

    Some Rails authentication systems have suffered from a vulnerability
    involving nil or blank login tokens:
    
      http://www.rorsecurity.info/2007/10/28/restful_authentication-login-security/
    
    This patch includes a bunch of test cases testing for possible attacks
    along these lines, and some sanity-checking code in our authentication
    methods.
    
    Note that the tests and the code don't really "line up" here--most of
    the test methods passed already, and most of the sanity-checking code
    is probably unnecessary.  But again, better safe than sorry.
    emk committed Dec 20, 2008
  6. @emk

    Security: Force all GET requests to be read-only

    The W3C makes a clear distinction between GET and POST requests.  GET
    requests should only cause "safe" actions, and the user should never be
    held accountable for making GET requests.  See the following for an
    overview:
    
      http://www.w3.org/2001/tag/doc/whenToUseGet.html
    
    The Rails 'protect_against_forgery' function (and possibly some web
    browsers) rely on the distinction between GET and POST to provide
    protection against CSRF attacks.  See:
    
      http://en.wikipedia.org/wiki/Cross-site_request_forgery
      http://guides.rubyonrails.org/security.html#_csrf_countermeasures
    
    Unfortunately, enforcing these rules in rather difficult, especially in
    a large application with lots of controllers and plugins.  So this patch
    applies a rather heavy-handed fix: We globally block database writes
    during GET requests, and specifically override that policy in one or two
    places.
    
    All of our current overrides invoke User#reset_token!.  I haven't
    performed a full security analysis of allowing User#reset_token! (or
    updates to session[:user] based on our "remember me" token) in a GET
    request.  For now, I'm going to go ahead and allow this activity--if we
    actually have some sort of vulnerability here, it affects a wide range
    of web applications.
    
    Note that this patch may break some part of the /admin interface.  I've
    tried posting articles and other basic stuff, but I haven't used the
    lesser-known corners of /admin since making these changes.  Please
    report any problems.
    emk committed Dec 20, 2008
  7. @emk

    Add unit test for img filtering

    Test case for a644733.
    emk committed Dec 20, 2008
  8. @emk

    Merge branch 'experimental'

    Conflicts:
    
    	app/drops/comment_drop.rb
    emk committed Dec 19, 2008
  9. @emk

    Security: Block <img ... /> tags when sanitizing

    A whole class of CSRF attacks uses the img tag:
    
      <img src="/admin/account/action_that_allows_get" />
    
    This will invoke action_that_allows_get using a GET request and first-
    party cookies.  There are some examples on Wikipedia:
    
      http://en.wikipedia.org/wiki/Cross-site_request_forgery
    
    Note that really solid enforcement of the "use GET only for queries"
    rule will also prevent this kind of attack.  Also note that if you
    allow third-party cookies, this patch doesn't help you at all--any
    other site on the Internet could trigger this attack.
    emk committed Dec 19, 2008
  10. @emk

    Security: Fix many broken filter regexps

    In Ruby, "foo\nbar" =~ /^bar/ will result in a match, because ^ matches
    at the start of any line, not at the start of the string.  In general,
    we want to use \A and \z in place of ^ and $, respectively.
    
    We rely heavily on regular expressions to filter untrusted data.  And
    many of these regular expressions can be fooled easily because they rely
    on ^ and $ when they shouldn't.  See comment_drop_test for a
    user-exploitable example.
    
    This patch does a bulk search-and-replace of the offending patterns.  It
    may easily have missed something somewhere, but it's a good start.
    emk committed Dec 19, 2008
Commits on Dec 19, 2008
  1. @emk

    Attempt to escape strings in FlickrMacro

    This plugin had absolutely no HTML escaping.  I've attempted to fix
    this, but since the API key has expired, this code has not been tested.
    emk committed Dec 19, 2008
  2. @emk

    Only accept known comment fields

    The comment form is most-exposed attack point in all of Mephisto,
    because it doesn't require an authenticity_token and can be used by
    anybody on the internet.
    
    In the interests of paranoia, this patch removes bulk assignment from
    the comment-posting code.  I don't see any way to exploit the previous
    code (several attr_accessible fields looked vulnerable, but don't
    actually exist any more).  But better safe than sorry.
    emk committed Dec 19, 2008
  3. @emk

    Security: Fix XSS attack against new comment form

    WARNING: If you're one of the first people testing this commit, please
    use a backup database.
    
    How to reproduce: Create a new comment, and set all fields to
    <script>alert("Pwned")</script>.  Submit it.  You will see a JavaScript
    alert dialog, which is bad.
    
    What's happening: Untrusted fields in Comment objects are sanitized
    immediately before they're written to the database for the first time.
    But if validation fails, it leaves the application with an unsanitized
    comment object.  When the "can't submit comment" error is displayed,
    this unsanitized comment object can be passed straight throught to
    Liquid, which assumes that all HTML tags have been escaped.
    
    (This may look like "self XSS" attack only, but hostile pages can
    trigger it by tricking you into submitting a comment form back to your
    own site, preloaded with malicious data.)
    
    How we fix it: We make HTML escaping the responsibility of CommentDrop,
    not the Comment model.  This means that we need to unescape several
    existing fields in the database.
    
    Possible issues: This means that we're storing dangerous, untrusted data
    in our database, and that we need to rely on the proper use of 'h' and
    'CGI.escapeHTML'.  In the case of 'h', we're already using SafeERB, so
    insecure admin templates will be caught automatically, and dangerous
    data should never be sent to the user.  In the case of Liquid, we need
    to carefully examine our CommentDrop class to make sure that we're not
    passing any unescaped data through to the Liquid templates.  But this is
    a pretty manageable "proof obligation"--and remember that the old
    "sanitize on create" code actually suffered from XSS attacks, because it
    was too easy to do the sanitization in the wrong place.
    emk committed Dec 19, 2008
Commits on Dec 18, 2008
  1. @emk

    Ignore .DS_Store files

    Everybody keeps adding this ignore rule to their github branches, so
    let's just go ahead and add it to master .gitignore file.  The .DS_Store
    file is created by MacOS X.
    emk committed Dec 18, 2008
  2. @emk

    Update security audit TODO list

    emk committed Dec 18, 2008
  3. @emk
  4. @emk

    Add test cases verifying that SafeERB works

    If an upgrade to Rails breaks SafeERB, we'd like to find out.
    emk committed Dec 18, 2008
Commits on Dec 17, 2008
  1. @emk

    Test whether database adapter supports safe_erb

    We need to make sure that whatever data we receive from the database is
    properly marked as tainted.
    emk committed Dec 17, 2008
  2. @emk

    Add highly-experimental safe_erb support

    This should help prevent unescaped text from being displayed in ERB
    templates, which should in turn help prevent XSS attacks.  This code is
    based on the safe_erb plugin, written by Shinya Kasatani and updated by
    Matthew Bass, with a whole bunch of changes to better support Mephisto
    and Rails 2.2.
    
    v2:
      Freeze emk-safe_erb 0.1.1, with MySQL support
    emk committed Dec 17, 2008
  3. @isaackearse
  4. @isaackearse

    Change location of the default theme from themes/default => app/theme…

    …s/default, and ignore the themes folder
    
    Now you can just symlink the themes folder when deploying and not overwrite any changes.
    isaackearse committed Dec 17, 2008
Commits on Dec 16, 2008
  1. @emk
  2. @emk

    Unpack tzinfo into vendor/gems

    This is an experimental change: Can we just go ahead and bundle tzinfo?
    This should certainly simplify installation for our users.  But it also
    makes it (slightly) harder to upgrade tzinfo when the timezones change.
    
    Let me know if this patch causes trouble.
    emk committed Dec 16, 2008
  3. @isaackearse
  4. @isaackearse

    require at least tzinfo 0.3.12

    Otherwise the Settings index action will blow up with the following error:
    ActionView::TemplateError: uninitialized constant TZInfo::Timezone::TimezoneProxy
    isaackearse committed Dec 17, 2008
  5. @emk

    Make 'rake gems' more robust when gems are missing

    When 'config.gem' fails to load a gem, Rails appears to go ahead and
    try to load routes anyway.  But in this scenario, the engines plugin
    doesn't get loaded either, and so we can't use it to load plugin routes.
    
    This patch attempts to make 'rake gems' a bit more robust in the face
    of these kinds of errors.  Unforunately, it may also cause mysterious
    failures of plugin routing if the engines plugin goes unloaded for
    other reasons, so I've included a warning.  This may not be entirely
    adequate, but it's better than breaking 'rake gems'.
    emk committed Dec 16, 2008
Commits on Dec 15, 2008
  1. @emk

    Rename forgot_password template to have correct MIME type

    This is part of some soon-to-be-committed work with safe_erb, but it
    stands alone just fine.
    emk committed Dec 15, 2008
Commits on Dec 14, 2008
  1. @emk

    Fix timezone bug caused by port to Rails 2.2

    This bug was introduced in 3414a37, and
    was reported by "barontick" on #mephisto.  Changes introduced while
    cleaning up unit test failures on the way to Rails 2.2 prevented users
    from setting the site's timezone.
    
    Some notes on this patch.
      1) We want to keep using 'tzinfo', and not Rail's built-in time zone
         classes, because the former supports daylight savings time.
      2) It's easier to just add a virtual timezone_name accessor instead of
         trying to do conversions in Site#timezone=.
      3) We can re-enable some old specs for Site, because there's no
         longer any danger of deleting site themes.
    
    I really wish we had a better way of testing that HTML forms could be
    submitted back to the database successfully, closing the loop between
    two different sets of test cases on output and input.
    emk committed Dec 14, 2008
Commits on Dec 13, 2008
  1. @isaackearse
Commits on Dec 12, 2008
  1. @isaackearse
  2. @emk

    Turn db:bootstrap:session into a regular file task

    The 'db:bootstrap' naming was inaccurate, and suggested that the task
    might (adversely) affect your database.  Instead, just turn this into
    an ordinary file task.  This also simplifies the code a bit.
    emk committed Dec 12, 2008
  3. @danlynn

    Fixed scoping issue and unexpected return issue.

    Fixed scoping issue where rake was aborting because of "uninitialized constant SecureRandom".
    
    Replaced return with conditional block to avoid aborting rake with an "unexpected return".
    danlynn committed Dec 12, 2008
  4. @emk

    Security: Replace white_list with Rails 2.2 sanitizer

    The Rails 2.2 santizer is an enhanced version of Rick's original
    white_list plugin, so let's upgrade and get the latest fixes.
    
    Note that Mephisto had separate rules for sanitizing comments and
    non-comments in Atom feeds.  This difference was introduced in commit
    88df87e.  Unfortunately, I'm not able
    to track down any information on the problem being fixed here.  Since we
    already add half of the tags in question to the whitelist, I've decided
    to just treat all sanitized Atom feed content the same.  Please let me
    know if this breaks anything.
    emk committed Dec 12, 2008
Something went wrong with that request. Please try again.