这是关于在代码审查中我们究竟想发现什么?系列6篇文章中的第6篇。
你在构建一个安全、稳定的系统所花的精力与你花在其他方面是一样的，这取决于项目的自身，项目运行在哪里、谁会使用它、什么样的数据会被访问，等等。团队中经常没有安全专家，我们可能在某个方向或其他方向上走偏：要么在安全上并不重视，要么就是弄一个大约20页左右的必须满足checklist，上面满是可能存在的潜在问题。我们现在来着重讨论下在code review中需要注意的地方，同时也一起讨论下在code review中团队应该注意哪些跟安全相关的问题。
1. 自动化是你的好伙伴
有惊人数量的安全检查可以自动化，因此应该是不需要人工参与的。安全测试不一定需要整个系统级的完整的渗透测试，有些安全问题可以再code-level被发现。
一些常见的SQL Injection或者Cross-site Scripting问题可以通过运行在CI环境上的工具来发现。你能够用OWASP Dependency Check 工具通过自动化检查一些依赖的已知漏洞。
例如，代码执行了动态生成的SQL字符串， 这可能会引起SQL Injection问题。
2. 有时候要“看情况”
对有的检验你可以很舒服的回答“Yes”或者“No”，有时你希望能有个工具来找出一些潜在的问题，然后有人来做出是否需要解决这个的决定。这也是Upsource可以闪光的地方。Upsource显示代码检查结果，reviewer可以使用它去决定代码是否需要修改或者在当前条件下是可以接受的。
例如，假设生成一个随机数，如果打开所有的安全检查点，你可以在Upsource里面看到下面的警告信息：
JavaDoc的java.util.Random特别申明“java.util.Random的实例并非密码学上安全的”。在大多数你需要一个任意的随机数的时候是没有问题的。但是如果你用它来生成session IDs,密码重置链接， nonces或者salts, review的时候可能需要建议用java.util.SecureRandom来替代它。
如果你和代码作者决定在这个情况是Random是可以的，那么你可以Suppress这个检查然后记录下为什么没有问题，或者把它指向这个主题的讨论来让将来的开发人员看这行代码的时候很容易理解你们做出这样深思熟虑的决定。
所以当工具能够发现潜在问题是，reviewer的其他部分工作就是调查自动化检查的结果和决定是否要采取行动处理这些问题。
3. 理解你的依赖
第三方类库是安全漏洞侵蚀你的系统或代码库的一个途径。当你review代码是，你至少需要检查是否有新的依赖（e.g. 第三方库）被引入。如果你没有通过自动化的方式去检查这些漏洞，你应该去检查那些新引入的类库中的已知安全问题。你同样应该尽可能的去最小化每个类库的版本，如果其他依赖有个额外的间接依赖，这个可能满足不了。但是一个最简单的方法使你的代码最小化暴露给其他人的代码中的安全问题是：
- 尽可能的使用源码，并理解它们可信度。
- 尽可能使用你能得到的最好质量的类库。
- 追踪你在何处使用了什么，当新的漏洞出现，你可以查看你受影响的程度。
4. 查看新的路径和服务是否需要认证
无论你是开发Web应用，或者提供Web服务还是一些需认证的API，当你新增一个URI或服务时，你需要确保这些新家的URI或服务在没有通过认证的情况下不能被访问（假设认证是你系统的需求）。你只需简单你查看开发者的代码里面是否写有合适的测试来展示认证是被应用的即可。
你需要考虑认证并不只针对是用户提供用户名与密码来做认证。身份信息也要针对其他的系统来定义或者是针对想访问你应用的自动化进程和服务，这个会影响你系统中的“用户”的概念。
5. 数据是否要加密
当你在磁盘上存储某些东西，或者通过网线发一些东西，你需要知道这些书序是否需要加密。很明显，密码永远都不能是简单文本，但是数据在很多情况下都需要加密。在code review时发现代码中有通过网线发数据，把数据存在某个地方或者通过什么方式离开你的系统，如果你不知道这些数据是否需要加密，尽可能找到在组织里能回答这个问题的人。
6. 秘密是否被正确的管理
像密码（用户密码，数据库或系统其他的密码），加密用的keys,token,这些都是秘密。这些绝对不能存在那些会被上传到源码控制系统中的代码里或者配置文件里面。可以有其它一些办法来管理它们，如，通过秘密服务器（secret server）。在code review时，要确保这些信息不会意外的悄悄的溜到你的版本控制系统中。
7. 代码运行是否该被记录或审计？做的正确与否？
工程的日志和审计要求各式各样，有些系统会有一些比其他系统都要严格的日志行为、事件规则。如果你有规章规定哪些需要记录日志，何时、如何记录，那么作为代码审查者你应该检查提交的代码是否满足要求。如果你没有固定的规章，那么就考虑：
- 代码是否改变了数据（如增删改操作）？是否应该记录由谁何时改变了什么？
- 代码是否涉及关键性能的部分？是否应该在性能监控系统中记录开始时间和结束时间？
- 每条日志的日志等级是否恰当？一个好的经验法则是“ERROR”触发一个提示发送到某处，如果你不想这些消息在凌晨3点叫醒谁，那么就将之降级为“INFO”或“DEBUG”。当在循环中或一条数据可能产生多条输出的情况下，一般不需要将它们记录到生产日志文件中，它们更应该被放在“DEBUG”级别。
8. 总结
-
记得叫上专家
安全是个很大的话题，大到足以让你的公司聘请技术安全专家。我们有安全专家就可以获得他们的帮助，如，邀请他们参加代码审查，或邀请他们在审查代码时和我们结对。如果这个无法实现，我们可以充分学习我们系统的环境，来理解我们有哪种安全需求（面向内部的企业级应用和面向客户的网页应用有不同的标准），所以我们可以更好地理解我们应该在代码审查中看什么。