A-A+

某站系统越权(可取消他人订单)代码分析

2016年12月09日 09:34 漏洞安全 评论 2 条 共1581字 (阅读3,371 views次)

【注意:此文章为博主原创文章!转载需注意,请带原文链接,至少也要是txt格式!】

具体某个程序我就不说了。但是这段代码看到后我发现不止是这段代码,底层也有问题。(其实这里面绕了一个弯路,我刚开始以为问题出在return,怎么想,都以为是绕过了return,但是无意中去测试拉粑粑,没带手机,,,然后突然就TM想明白了,跟if里面的return一点关系都没有,而是逻辑被绕过了。),下面是代码,先看看代码。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
/**
 * 取消订单
 * @param int
 * @return array
 */
public function actionCancelOrder(){
	$orderId = Usss::app()->request->getWaf('id');
	if(empty($orderId)){
		$result = array(
			'success'	=> false,
			'msg'	=> '请求参数有误!'
		);
		echo json_encode($result);
		return;
	}
	// 权限验证
	if (!$this->userId) {
		header("Location: http://gdd.gd/about");
		return;
	}
	$params = array(
		'orderId' => (int)$orderId,
		'userId' => (int)$this->userId,
		'orderType' => GxxxxHtel::ORDER_TYPE
	);
	$orderIso = new HotelOrderIso();
	$result = $orderIso->cancelOrder($params);
	if(empty($result)){
		$result = array(
			'success'	=> false,
			'msg'	=> 'unknown error'
		);
	}
	echo json_encode($result);
}

大家仔细看这段代码,正如上面说的,就因为这个注释,我硬生生以为程序走到了return,然后紧接着就继续走了,后来想通了。下面是我简易的分析。
首先获取订单的ID,然后判断这个订单ID是否为空,如果是则返回错误,如果不是则继续往下走!
下面有一个注释,是权限验证,就TM这个破JB注释,硬是让我以为这个地方没问题。通过$this->userId这个可以获取当前用户的ID,然后判断是否为空,如果不为空则往下走,如果为空,则相反,header跳转返回。
接下来就是全都装进数组(这里直接int了,所以不存在任何注入、XSS什么的,当然了本身有WAF也是不存在的),然后交给取消类里面的函数。然后判断返回值。最终返回JSON。
这里面有一个问题就是在权限验证这个地方,重点的重点就在于,它的逻辑是,判断是否获取到了当前用户的ID(也可以理解为,当前用户是否登陆,如果未登录当然获取不到当前用户的ID)。。。
再直白点解释,订单是用户A下的,但是当前取消订单的用户是B,而这个程序只判断了是否获取到了当前用户B的用户ID而已。如果获取到了,接着往下走就取消了。这里也就形成了越权。
经过跟踪发现它的cancelOrder也有问题,前端逻辑没验证,后端逻辑也没验证!这就导致了漏洞的发生。
最后给的修补建议是:
在IF语句之后填写如下代码:

1
2
3
4
5
6
7
8
$OrderModel = new OrderModel();
$arrObj = $OrderModel->getOrderDetail($params);
 
// 权限验证,实际userID不对,返回NULLs
if ($arrObj['userId'] != $this->userId ) {
	echo '您无权取消此订单!';
	exit;
}

先读取订单中下订单的用户ID在与当前用户ID做对比,如果不一致,那么则证明没权限。

这篇文字太简单了,自己都觉得简单的不要不要的,但是你知道么?很多很多很多大站,BR 9 BR8 PR7 PR8的站点就因为这个问题出现了很多很多的漏洞。
不多说了。虽然我不是程序员,但是干一行爱一行,写代码也应该用点心。

布施恩德可便相知重

微信扫一扫打赏

支付宝扫一扫打赏

×

2 条留言  访客:2 条  博主:0 条

  1. 路人甲

    有个疑问,
    $this->userid;
    这个是根据session来获取用户当前登录的ID还是根据前端传过来的id参数获取的呢?
    如果是根据session来获取当前用户ID的话,那就没有越权了吧,因为
    $params = array(
    ‘orderId’ => (int)$orderId,
    ‘userId’ => (int)$this->userId,
    ‘orderType’ => GxxxxHtel::ORDER_TYPE
    );
    $orderIso = new HotelOrderIso();
    $result = $orderIso->cancelOrder($params);
    这个$params参数获得的是当前用户的ID。
    除非cancelOrder函数里面没有校验userid用户是否是orderid这个订单的所有者

    • gdd

      是的 cancelOrder 里面没有验证订单所有者。

给我留言