Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

修复 csrf 图片退出登录漏洞

  • Loading branch information...
commit ad97b1d34c299cc6fb36b2d911cc6a548f179a75 1 parent 2afed4a
@alsotang alsotang authored
View
47 app.js
@@ -49,32 +49,29 @@ ndir.mkdir(config.upload_dir, function (err) {
var app = express.createServer();
// configuration in all env
-app.configure(function () {
- app.set('view engine', 'html');
- app.set('views', path.join(__dirname, 'views'));
- app.register('.html', require('ejs'));
- app.use(express.bodyParser({
- uploadDir: config.upload_dir
- }));
- app.use(express.cookieParser());
- app.use(express.session({
- secret: config.session_secret
- }));
- app.use(passport.initialize());
- // custom middleware
- app.use(require('./controllers/sign').auth_user);
- app.use(auth.blockUser());
- app.use('/upload/', express.static(config.upload_dir, { maxAge: maxAge }));
- // old image url: http://host/user_data/images/xxxx
- app.use('/user_data/', express.static(path.join(__dirname, 'public', 'user_data'), { maxAge: maxAge }));
-});
-
-app.configure('development', function () {
+app.set('view engine', 'html');
+app.set('views', path.join(__dirname, 'views'));
+app.register('.html', require('ejs'));
+app.use(express.bodyParser({
+ uploadDir: config.upload_dir
+}));
+app.use(express.methodOverride());
+app.use(express.cookieParser());
+app.use(express.session({
+ secret: config.session_secret
+}));
+app.use(passport.initialize());
+// custom middleware
+app.use(require('./controllers/sign').auth_user);
+app.use(auth.blockUser());
+app.use('/upload/', express.static(config.upload_dir, { maxAge: maxAge }));
+// old image url: http://host/user_data/images/xxxx
+app.use('/user_data/', express.static(path.join(__dirname, 'public', 'user_data'), { maxAge: maxAge }));
+
+if (config.debug) {
app.use('/public', express.static(staticDir));
app.use(express.errorHandler({ dumpExceptions: true, showStack: true }));
-});
-
-app.configure('production', function () {
+} else {
app.use(function (req, res, next) {
var csrf = express.csrf();
// ignore upload image
@@ -86,7 +83,7 @@ app.configure('production', function () {
app.use('/public', express.static(staticDir, { maxAge: maxAge }));
app.use(express.errorHandler());
app.set('view cache', true);
-});
+}
// set static, dynamic helpers
View
18 public/javascripts/main.js
@@ -1,14 +1,14 @@
$(document).ready(function () {
$('#search_form').submit(function (e) {
//e.preventDefault();
- search();
+ search();
});
-
+
function search() {
var q = document.getElementById('q');
if (q.value) {
/*
- var hostname = window.location.hostname;
+ var hostname = window.location.hostname;
var url = 'http://www.google.com/search?q=site:' + hostname + '%20';
window.open(url + q.value, '_blank');
*/
@@ -16,8 +16,8 @@ $(document).ready(function () {
} else {
return false;
}
- }
-
+ }
+
var $wrapper = $('#wrapper');
var $backtotop = $('#backtotop');
var $sidebar = $('#sidebar');
@@ -42,4 +42,12 @@ $(document).ready(function () {
$(window).resize(moveBacktotop);
$('.topic_content a,.reply_content a').attr('target', '_blank');
+
+ $('#signout').click(function () {
@JacksonTian Owner

这里为什么这么fix啊。

@cyrilis
cyrilis added a note

应该为了 RESTful 一点吧.

@alsotang Owner

想让 #signout 这个 a 标签被点击出 post 请求来。之前想用 form 包起来,但是实现很绕,所以改成这样了。
不懂有没有更好的实现方式?

@JacksonTian Owner
@cyrilis
cyrilis added a note

在以前版本的rails 中(不知道现在最新版本中还在不在), 有个官方 gem 包专门负责处理这些特定method请求的,https://github.com/rails/jquery-ujs

具体方法就是 给有特定 data-* 属性的元素添加绑定,比如

<a href="..." data-method="delete" rel="nofollow">Delete this entry</a>

这样 a 链接点击时发送delete请求。
这个是rails中的解决方法。

@alsotang Owner

get 的问题在于:图片请求的 scr 写成 signout 地址会导致退出。

@cyrilis 现在在 rails 中也存在,一般都是在 jquery 引用的正下方。

@JacksonTian Owner

不让这样的图片出现不就是了?

@alsotang Owner

你不觉得改成 post 请求是比 不让这样的图片出现 更好的方式吗?

@cyrilis
cyrilis added a note

这里有条回答
http://stackoverflow.com/questions/3521290/logout-get-or-post
说是有些浏览器会对链接进行预加载. 用 post方式可以避免浏览器预加载链接导致用户退出 , 当然从 RESTful 的角度来讲也不应该是 get 请求. 虽然绑定事件麻烦点儿, 但是这样明显更符合 RESTful 标准.

@alsotang Owner

我想了下

  1. 改成 csrf 应该是可以使安全性达到跟 post 一样的等级的。
  2. 但是就幂等性来说,这个 route 确实是 post 方式更合适。GitHub 是用 Rails 做的,用的是 jquery_ujs.js 里面的 data-method attribute 来实现 post

qq20140219-2

@alsotang Owner

@cyrilis 链接 good!

@JacksonTian Owner

核心问题是要防止伪造请求。不用太在意RESTful在这里的语意。

在logout链接末尾带个csrf参数:<a href="/logout?csrf=oxxxxxooooxxxxoo">Logout</a>
在服务器端,退出登录时需要验证这个csrf是否正确。这样就不用修改那么多代码了

@alsotang Owner

post 是需要坚持的。
代码难看的问题通过 rails 的解决方案搞定了:

58f10a2#commitcomment-5418003

@cyrilis
cyrilis added a note

@JacksonTian 也对, 关键是防止伪造请求.
@leizongmin 可是这样还是有预加载时导致用户退出的问题, 不过好像也不是常见的问题.

@cyrilis
cyrilis added a note

@alsotang Looks better now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $.post('/signout')
+ .done(function () {
+ window.location = '/';
+ });
+ return false;
+ });
});
View
2  routes.js
@@ -39,7 +39,7 @@ module.exports = function (app) {
} else {
app.get('/signup', configMiddleware.github, passport.authenticate('github'));
}
- app.get('/signout', sign.signout);
+ app.post('/signout', sign.signout);
app.get('/signin', sign.showLogin);
app.post('/signin', sign.login);
app.get('/active_account', sign.active_account);
View
2  views/layout.html
@@ -67,7 +67,7 @@
</li>
<li><a href='/setting'>设置</a></li>
<li>
- <a href='/signout'>退出</a>
+ <a href='/signout' id="signout">退出</a>
</li>
<% } else { %>
<li><a href='/signup'>注册</a></li>
Please sign in to comment.
Something went wrong with that request. Please try again.