New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source "X" can delete replies to source "Y" via the source interface #3892

Closed
heartsucker opened this Issue Oct 22, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@heartsucker
Copy link
Contributor

heartsucker commented Oct 22, 2018

Steps to reproduce

  1. Boot dev env
  2. Create source X, submit
  3. Open private tab, create source Y, submit
  4. Login as a journalist, reply to both
  5. In the /lookup page for source X, inspect the reply form and copy the hidden input reply_filename
  6. In the /lookup page for source Y, alter the reply form to have the copied value, submit/confirm
  7. Reload X's /lookup page and see that the reply is missing

Expected Behavior

X cannot delete Y's reply

Actual Behavior

X can delete Y's reply

Relevant code

securedrop/source_app/main.py

    @view.route('/delete', methods=('POST',))
    @login_required
    def delete():
        """This deletes the reply from the source's inbox, but preserves
        the history for journalists such that they can view conversation
        history.
        """

        query = Reply.query.filter(
            Reply.filename == request.form['reply_filename'])
        reply = get_one_or_else(query, current_app.logger, abort)
        reply.deleted_by_source = True
        db.session.add(reply)
        db.session.commit()

        flash(gettext("Reply deleted"), "notification")
        return redirect(url_for('.lookup'))

Looking at the Reply.query.filter, you can see that we do not check that a given response belongs to a source.

git blame

Not blaming, but listing it here for the eventual report to save y'all some time.

This was introduced in commit 83d2329b77 on 2015-02-15.

Analysis

Any authenticated source can delete replies belonging to any other source. However, they must know the filename, but we don't throttle anything on the source interface so a determined attacker could in theory disrupt source/journalist communication.

Estimated entropy of the filename:

>>> # num * nouns * adjectives * {doc,msg}
>>> math.log(100 *17949 * 8222 * 2, 2)
34.780745694714774

It seems feasible that a determined attacker could disrupt communications with some frequency on a high traffic instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment