某站系统越权(可取消他人订单)代码分析
【注意:此文章为博主原创文章!转载需注意,请带原文链接,至少也要是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的站点就因为这个问题出现了很多很多的漏洞。
不多说了。虽然我不是程序员,但是干一行爱一行,写代码也应该用点心。
有个疑问,
$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这个订单的所有者
是的 cancelOrder 里面没有验证订单所有者。