Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Turn on CSRF protection #63

Merged
merged 1 commit into from
Feb 25, 2014
Merged

Turn on CSRF protection #63

merged 1 commit into from
Feb 25, 2014

Conversation

jlfwong
Copy link
Member

@jlfwong jlfwong commented Feb 25, 2014

Flow has been CSRF vulnerable since forever ago 😄

Here's an example CSRF attack that when loaded in a user's browser while they're logged into flow will add the Security course to their shortlist.

<!-- uwflow-xsrf.html -->
<style type="text/css">
iframe {
    position: absolute;
    top: 0;
    left: -9000px;
}
</style>
<h1>Nothing Suspicious Here</h1>
<iframe src="uwflow-xsrf-frame.html"></iframe>
<!-- uwflow-xsrf-frame.html -->
<script>
window.onload = function() {
    var form = document.getElementById("frm");
    form.submit();
};
</script>
<form action="http://localhost:5000/api/user/add_course_to_shortlist" method="POST" id="frm">
    <input type="hidden" name="course_id" value="cs458" />
    <input type="submit" />
</form>

Test Plan

Loaded the above POC exploit before this patch, saw that it added the security course.

Applied patch, deleted account using tools/devshell.py, recreated account, ensured that I was still able to import transcript and add ratings to courses.

Loaded the above POC again, saw that it hit a 403 by looking at the network panel in the chrome devtools.

# and generally be more Rails-like
if flask.request.method != "GET":
# We intentionally don't invalidate CSRF tokens after a single use to
# enable multiple AJAX requests orgiinating from the page load to all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: originating

@divad12
Copy link
Member

divad12 commented Feb 25, 2014

LGTM, thanks!

jlfwong added a commit that referenced this pull request Feb 25, 2014
@jlfwong jlfwong merged commit ecffc02 into master Feb 25, 2014
@jlfwong jlfwong deleted the jlfwong-csrf branch February 25, 2014 23:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants