给一个电商开发的系统排查,发现漏洞很多。很多经验不够的开发者很容易忽视的逻辑错误陷阱。在给一个项目做二次开发时候,检测到的相关经典案例。这里整理支付和产品相关的逻辑,方便后续查看。,这里进行一些简单的逻辑漏洞梳理与修复。
这里基本可以看出经验少的程序员和有经验的程序员的差距,这种差距与算法能力无关,只与类似的系统设计经验有关,所以如果不是算法岗,盲目追逐算法,并不是最佳解。如果仅仅是以完成功能和对外展示流程走通为核心目标,代码逻辑质量看不出来,但是到真正运行的时候,有经验和无经验写的代码差距巨大。
1.原来逻辑与想法::
根据优惠券id,读取根据id状态下未使用的优惠券,进入下一步逻辑。
查询条件 id = $coupon_id + status = 0 读取一个未使用的优惠券。因为优惠券id是唯一的,再读取未使用的优惠券,俩个状态都符合,肯定表示该优惠券是可以使用的。
诊断风险:,读取优惠券之前,必须加上身份限定user_id 也就是需要三方条件成立,这张查询条件才是有效的。虽然大部分正常情况下,俩者查询结果一致,但是如果有恶意攻击者,从A账户,获取到了B账户的优惠券,这样就可以根据B的优惠券id 放入A账户使用。很显然,id = $coupon_id and user_id and status =0 查询安全性上明显高一截,当然另外的写法是全部读取出来,然后用业务代码判断,这会增加业务逻辑代码复杂度。
2.原始写这个系统人代码逻辑:
读取一个product_id 然后进行一次检查,又读取另外一个package_id 又进行一次检查,如果没有检测到相关信息,报错。每次读取一个参数,判断一次,不行,退出报错,没有问题。方便理解。
代码风险: 不是统一先验证必填参数的空参,就直接先读取数据库,如果后面又必填的参数是空参,会导致前面的查询都是浪费。成熟的写法,都是先对所有的必填参数进行验证,出现必填的空参,直接报错,让客户端传递完整参数。可以节约性能。
3.原始写法:API接口直接完成全部逻辑,就是一个接口逻辑从头写到尾,除了使用了内部Model封装的CRUD小方法,其他逻辑全部堆积到一个主方法里面,导致下单方法长度超过300行。
诊断风险: 大方法里面包含 优惠券使用逻辑,拼单逻辑,产品判断逻辑,下单的判断逻辑,产品的规则逻辑,要测试里面任意逻辑,都只能跑全部的方法,导致修改调试逻辑,只能跑整个方法,而整个方法,又因为参数过多,导致调试的效率大幅度降低。
改进方法: 将优惠券逻辑 结算逻辑 产品判断逻辑,规则逻辑 ,权限判断逻辑,全部拆开到model里面或者子方法里面,调试的时候,只需要填入我们要调试的基础逻辑即可,不需要跑其他已经正确的逻辑。
4.API 没有任何防刷,面对并发没有任何防御措施。一旦出现需要连续快速下单的情况,会造成用户可以使用俩次优惠券。查询的接口对防刷没有太高要求,但是对于数据有改动,账户金额有变动/优惠券有数量限制的 ,没有并发,基本就会被灰产使用。写该方法的人,应该没有被灰产攻击的经验,没有代码上有这种防御接口。
改进方法:其实主要还是防止用户网络不畅,连续点了俩次的情况出现。新增一个订单锁,确定该订单下单逻辑全部走完之后,才能下第二单。也就是增加一个流程锁,每个用户在一个时间只能下一单,防止用户的优惠和余额关键信息被同时改动,导致数据失效。一般普通流程锁,使用redis的set即可完成,如果是存在更高级的并发的,需要使用到setnx 。流程锁需要注意的问题是,在执行完成全部逻辑后,需要进行对应解锁。所以最好的执行方法就是,controllerl里面只操作参数判断+锁流程+主流程,而流程放到logic层或者model层。
5.当前的写法:没有产品和店铺的salesnum字段,需要的时候,直接采用 count的方法读取数据库,简单方便,实时读取数据库。很多新手设计表的时候,特别喜欢使用count来替代统计功能,方便好用,也容易理解。
风险警示:一旦遇到诸如需要后台统计每个产品的销量还有,每个店铺的销售量的时候,只能循环使用count,对系统的性能是噩梦,特别是当订单数量达到一百万的时候,会发现刷下后台,就会导致整个系统卡死。
改进方法:对全部需要统计的东西,尽量额外增加一个count_num字段,在进行改动的时候对该字段进行+1 或者-1 如果对精确度要求不高,就不使用事物。否则就需要事物来控制准确度。