Skip to content

Commit

Permalink
[MERGE] CSRF protection support in routes and forms
Browse files Browse the repository at this point in the history
* make CSRF protection the default on all non-SAFE methods
  (any method besides GET, HEAD, TRACE and OPTIONS )
  note: there currently is no way to call a CSRF-protected endpoint
  without a form-encoded entity-body as that's the only place we get
  the CSRF token from.
* simple CSRF token generation: just use the HMAC'd session id, no
  generating a new random token per session then HMAC it
* use constant-time equal function to avoid timing attacks
* assert that a database secret is configured before
  hashing/validating the CSRF token
* opt-out database manager from CSRF: the super-admin password
  serves the purpose of a CSRF token in the database manager screens.

Implementation by [email protected] and [email protected]
  • Loading branch information
odony committed Oct 1, 2015
2 parents 88b63a9 + afa9f4b commit 8932c52
Show file tree
Hide file tree
Showing 41 changed files with 207 additions and 70 deletions.
2 changes: 2 additions & 0 deletions addons/auth_signup/views/auth_signup_login.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
<template id="auth_signup.signup" name="Sign up login">
<t t-call="web.login_layout">
<form class="oe_signup_form" role="form" method="post" t-if="not message">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>

<t t-call="auth_signup.fields">
<t t-set="only_passwords" t-value="bool(token)"/>
Expand Down Expand Up @@ -67,6 +68,7 @@
</div>

<form class="oe_reset_password_form" role="form" method="post" t-if="not message">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>

<t t-if="token">
<t t-call="auth_signup.fields">
Expand Down
2 changes: 1 addition & 1 deletion addons/base_import/controllers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from openerp.http import Controller, route

class ImportController(Controller):
@route('/base_import/set_file')
@route('/base_import/set_file', methods=['POST'])
def set_file(self, req, file, import_id, jsonp='callback'):
import_id = int(import_id)

Expand Down
1 change: 1 addition & 0 deletions addons/base_import/static/src/xml/import.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<t t-name="ImportView">
<t t-set="_id" t-value="_.uniqueId('export')"/>
<form action="" method="post" enctype="multipart/form-data" class="oe_import">
<input type="hidden" name="csrf_token" t-att-value="csrf_token"/>
<input type="hidden" name="session_id"
t-att-value="widget.session.session_id"/>
<input type="hidden" name="import_id"/>
Expand Down
2 changes: 1 addition & 1 deletion addons/payment_ogone/controllers/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def ogone_s2s_create_json(self, **kwargs):
new_id = acquirer.s2s_process(data)
return new_id

@http.route(['/payment/ogone/s2s/create'], type='http', auth='public')
@http.route(['/payment/ogone/s2s/create'], type='http', auth='public', methods=["POST"])
def ogone_s2s_create(self, **post):
acquirer_id = int(post.get('acquirer_id'))
acquirer = request.env['payment.acquirer'].browse(acquirer_id)
Expand Down
1 change: 1 addition & 0 deletions addons/payment_ogone/views/ogone.xml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@

<template id="ogone_s2s_form">
<form method="post" t-att-action="'/payment/ogone/s2s/create' if not json else '/payment/ogone/s2s/create_json'">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
<div t-att-class="'row' if bootstrap_formatting else ''">
<div t-att-class="'form-group col-md-8' if bootstrap_formatting else 'form-group'">
<label class="control-label" for="cc_number">Card number</label>
Expand Down
2 changes: 1 addition & 1 deletion addons/payment_transfer/controllers/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class OgoneController(http.Controller):

@http.route([
'/payment/transfer/feedback',
], type='http', auth='none')
], type='http', auth='none', csrf=False)
def transfer_form_feedback(self, **post):
cr, uid, context = request.cr, SUPERUSER_ID, request.context
_logger.info('Beginning form_feedback with post data %s', pprint.pformat(post)) # debug
Expand Down
2 changes: 1 addition & 1 deletion addons/payment_transfer/models/payment_acquirer.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# -*- coding: utf-'8' "-*-"
# -*- coding: utf-8 -*-

from openerp.addons.payment.models.payment_acquirer import ValidationError
from openerp.osv import osv
Expand Down
4 changes: 2 additions & 2 deletions addons/payment_transfer/views/transfer.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@

<template id="transfer_acquirer_button">
<form t-if="acquirer" t-att-action="tx_url" method="post" target="_self">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
<t t-if="return_url">
<input type='hidden' name='return_url' t-att-value='return_url'/>
</t>
<input type='hidden' name='reference' t-att-value='reference'/>
<input type='hidden' name='amount' t-att-value='amount'/>
<input type='hidden' name='currency' t-att-value='currency.name'/>
<!-- submit -->
<button type="submit" width="100px"
t-att-class="submit_class">
<button type="submit" width="100px" t-att-class="submit_class">
<img t-if="not submit_txt" src="/payment_transfer/static/src/img/transfer_icon.png"/>
<span t-if="submit_txt"><t t-esc="submit_txt"/> <span class="fa fa-long-arrow-right"/></span>
</button>
Expand Down
2 changes: 1 addition & 1 deletion addons/survey/controllers/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def start_survey(self, survey, token=None, **post):
# Survey displaying
@http.route(['/survey/fill/<model("survey.survey"):survey>/<string:token>',
'/survey/fill/<model("survey.survey"):survey>/<string:token>/<string:prev>'],
type='http', auth='public', website=True)
type='http', auth='public', website=True, methods=['POST'])
def fill_survey(self, survey, token, prev=None, **post):
'''Display and validates a survey'''
cr, uid, context = request.cr, request.uid, request.context
Expand Down
5 changes: 3 additions & 2 deletions addons/survey/views/survey_templates.xml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@
</div>

<form role="form" method="post" class="js_surveyform" t-att-name="'%s_%s' % (survey.id, page.id)" t-att-action="'/survey/fill/%s/%s' % (slug(survey), token)" t-att-data-prefill="'/survey/prefill/%s/%s/%s' % (slug(survey), token, slug(page))" t-att-data-validate="'/survey/validate/%s' % (slug(survey))" t-att-data-submit="'/survey/submit/%s' % (slug(survey))">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
<input type="hidden" name="page_id" t-att-value="page.id" />
<input type="hidden" name="token" t-att-value="token" />
<t t-foreach='page.question_ids' t-as='question'>
Expand Down Expand Up @@ -285,7 +286,7 @@
<h1><span t-field='survey.title'/></h1>
<t t-if="survey.description"><div t-field='survey.description' class="oe_no_empty"/></t>
</div>
<form role="form" method="post" class="js_surveyform" t-att-name="'%s' % (survey.id)" t-att-data-prefill="'/survey/prefill/%s/%s' % (slug(survey), token)" t-att-data-scores="'/survey/scores/%s/%s' % (slug(survey), token) if quizz_correction else ''">
<div role="form" class="js_surveyform" t-att-name="'%s' % (survey.id)">
<t t-foreach="survey.page_ids" t-as="page">
<div class="page-header">
<h1 t-field='page.title' />
Expand All @@ -312,7 +313,7 @@
</t>
<hr/>
</t>
</form>
</div>
</div>
</div>
</div>
Expand Down
18 changes: 6 additions & 12 deletions addons/web/controllers/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,12 +565,6 @@ def web_login(self, redirect=None, **kw):
values['error'] = "Wrong login/password"
return request.render('web.login', values)

@http.route('/login', type='http', auth="none")
def login(self, db, login, key, redirect="/web", **kw):
if not http.db_filter([db]):
return werkzeug.utils.redirect('/', 303)
return login_and_redirect(db, login, key, redirect_url=redirect)

class WebClient(http.Controller):

@http.route('/web/webclient/csslist', type='json', auth="none")
Expand Down Expand Up @@ -736,7 +730,7 @@ def selector(self, **kw):
def manager(self, **kw):
return self._render_template()

@http.route('/web/database/create', type='http', auth="none")
@http.route('/web/database/create', type='http', auth="none", methods=['POST'], csrf=False)
def create(self, master_pwd, name, lang, password, **post):
try:
request.session.proxy("db").create_database(master_pwd, name, bool(post.get('demo')), lang, password)
Expand All @@ -746,7 +740,7 @@ def create(self, master_pwd, name, lang, password, **post):
error = "Database creation error: %s" % e
return self._render_template(error=error)

@http.route('/web/database/duplicate', type='http', auth="none")
@http.route('/web/database/duplicate', type='http', auth="none", methods=['POST'], csrf=False)
def duplicate(self, master_pwd, name, new_name):
try:
request.session.proxy("db").duplicate_database(master_pwd, name, new_name)
Expand All @@ -755,7 +749,7 @@ def duplicate(self, master_pwd, name, new_name):
error = "Database duplication error: %s" % e
return self._render_template(error=error)

@http.route('/web/database/drop', type='http', auth="none")
@http.route('/web/database/drop', type='http', auth="none", methods=['POST'], csrf=False)
def drop(self, master_pwd, name):
try:
request.session.proxy("db").drop(master_pwd, name)
Expand All @@ -764,7 +758,7 @@ def drop(self, master_pwd, name):
error = "Database deletion error: %s" % e
return self._render_template(error=error)

@http.route('/web/database/backup', type='http', auth="none")
@http.route('/web/database/backup', type='http', auth="none", methods=['POST'], csrf=False)
def backup(self, master_pwd, name, backup_format = 'zip'):
try:
openerp.service.db.check_super(master_pwd)
Expand All @@ -782,7 +776,7 @@ def backup(self, master_pwd, name, backup_format = 'zip'):
error = "Database backup error: %s" % e
return self._render_template(error=error)

@http.route('/web/database/restore', type='http', auth="none")
@http.route('/web/database/restore', type='http', auth="none", methods=['POST'], csrf=False)
def restore(self, master_pwd, backup_file, name, copy=False):
try:
data = base64.b64encode(backup_file.read())
Expand All @@ -792,7 +786,7 @@ def restore(self, master_pwd, backup_file, name, copy=False):
error = "Database restore error: %s" % e
return self._render_template(error=error)

@http.route('/web/database/change_password', type='http', auth="none")
@http.route('/web/database/change_password', type='http', auth="none", methods=['POST'], csrf=False)
def change_password(self, master_pwd, master_pwd_new):
try:
request.session.proxy("db").change_admin_password(master_pwd, master_pwd_new)
Expand Down
17 changes: 10 additions & 7 deletions addons/web/static/src/js/framework/ajax.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
odoo.define('web.ajax', function (require) {
"use strict";

var core = require('web.core');
var time = require('web.time');
var utils = require('web.utils');

Expand Down Expand Up @@ -238,6 +239,11 @@ function get_file(options) {
method: 'POST'
}).appendTo(document.body);
}
if (core.csrf_token) {
$('<input type="hidden" name="csrf_token">')
.val(core.csrf_token)
.appendTo($form_data);
}

var hparams = _.extend({}, options.data || {}, {token: token});
_.each(hparams, function (value, key) {
Expand Down Expand Up @@ -292,19 +298,16 @@ function post (controller_url, data) {
};

var Def = $.Deferred();
var compatibility = !(typeof(FormData));
var postData = compatibility ? new FormDataCompatibility() : new FormData();
var postData = new FormData();

$.each(data, function(i,val) {
postData.append(i, val);
});

var xhr = new XMLHttpRequest();
if(compatibility) {
postData.setContentTypeHeader(xhr);
postData = postData.buildBody();
if (core.csrf_token) {
postData.append('csrf_token', core.csrf_token);
}

var xhr = new XMLHttpRequest();
if(xhr.upload) xhr.upload.addEventListener('progress', progressHandler(Def), false);

var ajaxDef = $.ajax(controller_url, {
Expand Down
3 changes: 2 additions & 1 deletion addons/web/static/src/xml/base.xml
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,8 @@
<t t-name="HiddenInputFile">
<div t-attf-class="oe_hidden_input_file #{fileupload_class or ''}" t-att-style="fileupload_style">
<form class="o_form_binary_form" t-att-target="fileupload_id"
method="post" enctype="multipart/form-data" t-att-action="fileupload_action || '/web/binary/upload'">
method="post" enctype="multipart/form-data" t-att-action="fileupload_action || '/web/binary/upload'">
<input type="hidden" name="csrf_token" t-att-value="csrf_token"/>
<input type="hidden" name="session_id" value="" t-if="widget.session.override_session"/>
<input type="hidden" name="callback" t-att-value="fileupload_id"/>
<t t-raw="0"/>
Expand Down
8 changes: 8 additions & 0 deletions addons/web/views/webclient_templates.xml
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@
<t t-call="web.login_layout">

<form class="oe_login_form" role="form" t-attf-action="/web/login{{ '?debug' if debug else '' }}" method="post" onsubmit="this.action = this.action + location.hash">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>

<div class="form-group field-db" t-if="databases and len(databases) &gt; 1">
<label for="db" class="control-label">Database</label>
Expand Down Expand Up @@ -300,6 +301,13 @@
<t t-call="web.layout">
<t t-set="head">
<t t-call-assets="web.assets_common"/>
<script type="text/javascript">
odoo.define('web.csrf', function (require) {
var token = "<t t-esc="request.csrf_token(None)"/>";
require('web.core').csrf_token = token;
require('qweb').default_dict.csrf_token = token;
});
</script>
<t t-call-assets="web.assets_backend"/>
&lt;!--[if lte IE 9]&gt; <link rel="stylesheet" href="/web/static/src/css/ie.css"/> &lt;![endif]--&gt;
<script type="text/javascript">
Expand Down
1 change: 1 addition & 0 deletions addons/web_editor/static/src/xml/editor.xml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
enctype="multipart/form-data"
target="fileframe"
class="form-inline">
<input type="hidden" name="csrf_token" t-att-value="csrf_token"/>
<div class="well">
<div class="form-group pull-left">
<input type="file" name="upload" t-att-accept="widget.accept" multiple="multiple" style="position: absolute; opacity: 0; width: 1px; height: 1px;"/>
Expand Down
1 change: 1 addition & 0 deletions addons/website/data/demo.xml
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ response = request.website.render("website.template_partner_post", values)
<div class="container">
<div class="row">
<form class="form-horizontal" action="/website/action/website.action_partner_post" method="post">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
<div class="form-group">
<label class="col-sm-2 control-label">Recipient</label>
<div class="col-sm-10">
Expand Down
17 changes: 12 additions & 5 deletions addons/website/static/src/js/website.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,23 @@ var error = function(data, url) {
$error.modal('show');
};

function _add_input(form, name, value) {
var param = document.createElement('input');
param.setAttribute('type', 'hidden');
param.setAttribute('name', name);
param.setAttribute('value', value);
form.appendChild(param);
}
var form = function (url, method, params) {
var form = document.createElement('form');
form.setAttribute('action', url);
form.setAttribute('method', method);

if (core.csrf_token) {
_add_input(form, 'csrf_token', core.csrf_token);
}
_.each(params, function (v, k) {
var param = document.createElement('input');
param.setAttribute('type', 'hidden');
param.setAttribute('name', k);
param.setAttribute('value', v);
form.appendChild(param);
_add_input(form, k, v);
});
document.body.appendChild(form);
form.submit();
Expand Down
1 change: 1 addition & 0 deletions addons/website/static/src/xml/website.gallery.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
</div>
<div class="modal-body">
<form action="/web_editor/attachment/add" method="post" enctype="multipart/form-data" >
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
<div class="form-group">
<input name="upload" type="file" value="Choose images" multiple="multiple" accept="image/*"/>
</div>
Expand Down
8 changes: 8 additions & 0 deletions addons/website/views/website_templates.xml
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,13 @@
<t t-call-assets="website.assets_frontend" t-js="false"/>
<t t-call-assets="web.assets_common" t-css="false"/>
<t t-call-assets="website.assets_frontend" t-css="false"/>
<script type="text/javascript">
odoo.define('web.csrf', function (require) {
var token = "<t t-esc="request.csrf_token(None)"/>";
require('web.core').csrf_token = token;
require('qweb').default_dict.csrf_token = token;
});
</script>

<t t-raw="head or ''" name='layout_head'/>
</head>
Expand Down Expand Up @@ -846,6 +853,7 @@
<p>An error occured while rendering the template <code t-esc="qweb_exception.qweb['template']"/>.</p>
<p>If this error is caused by a change of yours in the templates, you have the possibility to reset one or more templates to their <strong>factory settings</strong>.</p>
<form action="/website/reset_templates" method="post" id="reset_templates_form">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
<ul class="oe_template_fallback">
<li t-foreach="views" t-as="view">
<label>
Expand Down
2 changes: 1 addition & 1 deletion addons/website_blog/controllers/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ def blog_post_create(self, blog_id, **post):
new_blog_post = request.registry['blog.post'].browse(cr, uid, new_blog_post_id, context=context)
return werkzeug.utils.redirect("/blog/%s/post/%s?enable_editor=1" % (slug(new_blog_post.blog_id), slug(new_blog_post)))

@http.route('/blog/post_duplicate', type='http', auth="public", website=True)
@http.route('/blog/post_duplicate', type='http', auth="public", website=True, methods=['POST'])
def blog_post_copy(self, blog_post_id, **post):
""" Duplicate a blog.
Expand Down
9 changes: 5 additions & 4 deletions addons/website_blog/views/website_blog_templates.xml
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,11 @@
<t t-set="object" t-value="blog_post"/>
<t t-set="publish_edit" t-value="True"/>
<li>
<form class="duplicate hidden" action="/blog/post_duplicate">
<input name="blog_post_id" t-att-value="blog_post.id"/>
</form>
<a href="#" class="duplicate" onclick="$(this).prev('form').submit()">Duplicate</a>
<form class="duplicate hidden" action="/blog/post_duplicate" method="POST">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
<input name="blog_post_id" t-att-value="blog_post.id"/>
</form>
<a href="#" class="duplicate" onclick="$(this).prev('form').submit()">Duplicate</a>
</li>
</t>
</div>
Expand Down
1 change: 1 addition & 0 deletions addons/website_crm/views/website_crm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<xpath expr="//div[@name='mail_button']" position="replace">
<div>
<form action="/website_form/" method="post" data-model_name="crm.lead" data-success_page="/page/website_crm.contactus_thanks" class="s_website_form form-horizontal container-fluid mt32" enctype="multipart/form-data">
<input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
<div class="form-group form-field o_website_form_required_custom">
<label class="col-md-3 col-sm-4 control-label" for="contact_name">Your Name</label>
<div class="col-md-7 col-sm-8">
Expand Down
4 changes: 2 additions & 2 deletions addons/website_event/static/src/js/website_geolocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ var animation = require('web_editor.snippets.animation');
animation.registry.visitor = animation.Class.extend({
selector: ".oe_country_events",
start: function () {
$.post( "/event/get_country_event_list", function( data ) {
$.get("/event/get_country_event_list").then(function( data ) {
if(data){
$( ".country_events_list" ).replaceWith( data );
}
});
}
});

});
});
Loading

0 comments on commit 8932c52

Please sign in to comment.