Skip to content


Subversion checkout URL

You can clone with
Download ZIP
Commits on Mar 15, 2009
  1. @emk

    multisite: Fix broken article versions

    Spyridon Vasileiadis authored emk committed
    [This is an edited version of the author's blog post at -Eric]
    Currently in mephisto edge, article versions are broken. More precisely,
    the bug comes up ONLY when the installation operates in multisite mode
    (or even more precisely, when there are at least two articles belonging
    to two different sites).
    The problem is how acts_as_versioned is being used. An acts_as_versioned
    record has among others an “id” column (the default id that ActiveRecord
    requires) and a “version” column.
    Currently Mephisto falsely does the following inside
    \app\controllers\admin\articles_controller.rb on line 38 (edit
      @version = params[:version] ? @article.versions.find(params[:version]) : @article or raise(ActiveRecord::RecordNotFound)
    the whole problem is the find(params[:version]) . What happens here is,
    that we lookup an article’s version by searching for its id instead of
    for its version column (even though we do use the correct :version
    So this has to change to find_by_version(params[:version]) and thus
      @version = params[:version] ? @article.versions.find_by_version(params[:version]) : @article or raise(ActiveRecord::RecordNotFound)
    Notice though that this doesn’t break in a single-site installation,
    because in this case id and version bot get the same (concurrent)
    increment. That is because all articles belong the same one and only
    Site instance.
  2. @davec @emk

    Fix tainted string error when updating comment

    davec authored emk committed
    When updating a comment, a tainted string error was being thrown due to
    the lack of a h() escape on the article title in the comment partial.
    A deprecation warning from truncate() in the comment partial has also
    been resolved.
    A functional test (test_should_update_comment) for the admin comments
    controller is also included.
  3. @davec @emk

    Expire cache on theme change from admin/settings

    davec authored emk committed
    Force cache expiration when changing the theme from the Admin::Settings
    controller as is done when changing the theme from the Admin::Themes
    controller so that the behavior after a change of theme is consistent,
    regardless of where the change is made.
  4. @davec @emk

    Pass comment approval status to the template

    davec authored emk committed
    By passing the comment's approval setting to the __thanks_for_comment
    template, the template can provide additional feedback such as "Your
    comment is awaiting moderator approval", "Sorry, we threw your comment
    away", or similar if the comment was not automatically approved.  For
    example, __thanks_for_comment.liquid could look like the following:
      Thanks for the comment.
      {% unless approved %}It is awaiting moderation.{% endunless %}
  5. @davec @emk

    Force consistent ordering of dates containing NULLs

    davec authored emk committed
    Because database servers differ on how they sort NULL values, the sort
    order for articles was changed, via COALESCE, so that a NULL date will
    be treated as being less than (i.e., older than) any non-NULL date.
    This fixes test_should_search_article_by_section so that it passes with
    MySQL, SQLite, and PostgreSQL.
    [I'm merging this patch because I'm quite fond of PostgreSQL, and
    because it has been tested with most of the other databases we support.
    If this breaks your database, please let me know. -Eric]
  6. @webmat @emk

    Fix the parameter logging filtering...

    webmat authored emk committed
    Call filter_parameter_logging only once with all sensitive field names, rather than once per field to protect.
Commits on Feb 2, 2009
  1. @emk

    Fix display of theme homepage links

    emk authored
    Many thanks to Gustavo Sales (aka vatsu) for pointing out this bug and
    proposing a fix:
    In order to minimize code duplication in the *.erb files, I've rewritten
    this code as a helper method.
  2. @emk
Commits on Jan 30, 2009
  1. missing tainted string in cache listing

    Sean O'Brien authored
Commits on Jan 13, 2009
  1. @james2m @emk

    escaped link in _page.html.erb

    james2m authored emk committed
    Signed-off-by: James McCarthy <>
Commits on Jan 9, 2009
  1. @emk

    Changed user login to send user to admin section on succesful login i…

    Chris Cummer authored emk committed
    …nstead of the blog homepage since users have the ability to post to the blog
Commits on Dec 31, 2008
  1. @emk

    Fixes 'attempted to output tainted string' error when rendering email…

    Chris Cummer authored emk committed
    … address for mailto
Commits on Dec 26, 2008
  1. @emk
Commits on Dec 24, 2008
  1. @emk

    Modernize rjs: admin/articles

    emk authored
    I'm going to try to rename all the *.rjs files to *.js.rjs.  This is
    trickier than the other renamings, because our unit test coverage isn't
    perfect, and I'm trying to test everything by hand when possible.  So
    I'm going to do this one directory at a time.
    Other changes:
      - The live_preview and _preview stuff wasn't being used.
      - We didn't have a test case for the 'label' action.
  2. @emk

    Rename *.rxml files to *.xml.builder

    emk authored
    This also requires adding respond_to blocks to some of our actions.
  3. @emk

    Rename *.rhtml files to *.html.erb

    emk authored
    Let's go for the new-school approach here.
Commits on Dec 22, 2008
  1. @emk

    Fix theme controller bugs

    emk authored
    J.C. Zhu reported an InvalidAuthenticityToken error when trying to
    change themes.  This was one of several bugs in the theme switching
    code left over from the Rails 2.2 porting process and the security
    We needed to convince Ajax.Request to use a GET request when displaying
    the theme tools, and we needed to properly escape some text in the
    _tools template.
    I've also added a test case to make sure we actually render the _tools
    view successfully.
Commits on Dec 20, 2008
  1. @technoweenie
  2. @technoweenie
  3. @emk

    Don't log "remember me" token

    emk authored
    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.
  4. @emk

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

    emk authored
    Some Rails authentication systems have suffered from a vulnerability
    involving nil or blank login tokens:
    This patch includes a bunch of test cases testing for possible attacks
    along these lines, and some sanity-checking code in our authentication
    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.
  5. @emk

    Security: Force all GET requests to be read-only

    emk authored
    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
    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:
    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
    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.
  6. @emk

    Merge branch 'experimental'

    emk authored
  7. @emk

    Security: Fix many broken filter regexps

    emk authored
    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.
Commits on Dec 19, 2008
  1. @emk

    Only accept known comment fields

    emk authored
    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.
  2. @emk

    Security: Fix XSS attack against new comment form

    emk authored
    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.
Commits on Dec 17, 2008
  1. @isaackearse

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

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

    Rename forgot_password template to have correct MIME type

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

    Fix timezone bug caused by port to Rails 2.2

    emk authored
    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.
Commits on Dec 12, 2008
  1. @isaackearse
  2. @emk

    Security: Replace white_list with Rails 2.2 sanitizer

    emk authored
    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.
  3. @emk

    Security: More h(...) tags

    emk authored
    These are based on manual inspection.
  4. @emk

    Security: Escape more strings

    emk authored
    The sqlite3_ruby database adapter does not correctly taint strings
    unless you first apply this patch:
    With this patch applied, SafeERB finds more errors.  These are now
    fixed.  It would be highly desirable to update SafeERB and modify it for
    production use with Rails 2.2.
Commits on Dec 11, 2008
  1. @emk

    Security: Escape strings where recommended by SafeERB

    emk authored
    The SafeERB plugin attempts to automatically detect view code which
    fails to properly escape HTML.  You can find information here:
    I'm using a version of SafeERB modified by Matthew Bass, which can be
    found on github:
    My local copy is modified to avoid some false positives, and isn't ready
    for production use yet.  But here's the first batch of changes it
    recommended.  Note that some of these changes weren't really necessary--
    some of the values we're wrapping can't actually contain HTML
    metacharacters, at least not in normal locales.
    Also note that SafeERB is only useful for the normal view code in places
    like /admin, and that it won't help us with Liquid plugins in the front
    end.  But it's a start.
  2. @emk

    Security: Make 'token' cookie HTTP-only

    emk authored
    This prevents malicious JavaScript code injected by a XSS attack from
    reading your "Remember me" token, and getting long-term access to your
    account.  Note that not all browsers honor :http_only, and of those
    that do, some allow it to be bypassed using XmlHttpRequest.
Something went wrong with that request. Please try again.