2 不推荐的编程习惯

2.1 【必须】switch中应有default

switch中应该有default,以处理各种预期外的情况。这可以确保switch接受用户输入,或者后期在其他开发者修改函数后确保switch仍可以覆盖到所有情况,并确保逻辑正常运行。

  1. // Bad
  2. int Foo(int bar) {
  3. switch (bar & 7) {
  4. case 0:
  5. return Foobar(bar);
  6. break;
  7. case 1:
  8. return Foobar(bar * 2);
  9. break;
  10. }
  11. }

例如上述代码switch的取值可能从0~7,所以应当有default:

  1. // Good
  2. int Foo(int bar) {
  3. switch (bar & 7) {
  4. case 0:
  5. return Foobar(bar);
  6. break;
  7. case 1:
  8. return Foobar(bar * 2);
  9. break;
  10. default:
  11. return -1;
  12. }
  13. }

关联漏洞:

中风险-逻辑漏洞

中风险-内存泄漏

2.2 【必须】不应当在Debug或错误信息中提供过多内容

包含过多信息的Debug消息不应当被用户获取到。Debug信息可能会泄露一些值,例如内存数据、内存地址等内容,这些内容可以帮助攻击者在初步控制程序后,更容易地攻击程序。

  1. // Bad
  2. int Foo(int* bar) {
  3. if (bar && *bar == 5) {
  4. OutputDebugInfoToUser("Wrong value for bar %p = %d\n", bar, *bar);
  5. }
  6. }

而应该:

  1. // Good
  2. int foo(int* bar) {
  3. #ifdef DEBUG
  4. if (bar && *bar == 5) {
  5. OutputDebugInfo("Wrong value for bar.\n", bar, *bar);
  6. }
  7. #endif
  8. }

关联漏洞:

中风险-信息泄漏

2.3 【必须】不应该在客户端代码中硬编码对称加密秘钥

不应该在客户端代码中硬编码对称加密秘钥。例如:不应在客户端代码使用硬编码的 AES/ChaCha20-Poly1305/SM1 密钥,使用固定密钥的程序基本和没有加密一样。

如果业务需求是认证加密数据传输,应优先考虑直接用 HTTPS 协议。

如果是其它业务需求,可考虑由服务器端生成对称秘钥,客户端通过 HTTPS 等认证加密通信渠道从服务器拉取。

或者根据用户特定的会话信息,比如登录认证过程可以根据用户名用户密码业务上下文等信息,使用 HKDF 等算法衍生出对称秘钥。

又或者使用 RSA/ECDSA + ECDHE 等进行认证秘钥协商,生成对称秘钥。

  1. // Bad
  2. char g_aes_key[] = {...};
  3. void Foo() {
  4. ....
  5. AES_func(g_aes_key, input_data, output_data);
  6. }

可以考虑在线为每个用户获取不同的密钥:

  1. // Good
  2. char* g_aes_key;
  3. void Foo() {
  4. ....
  5. AES_encrypt(g_aes_key, input_data, output_data);
  6. }
  7. void Init() {
  8. g_aes_key = get_key_from_https(user_id, ...);
  9. }

关联漏洞:

中风险-信息泄露

2.4 【必须】返回栈上变量的地址

函数不可以返回栈上的变量的地址,其内容在函数返回后就会失效。

  1. // Bad
  2. char* Foo(char* sz, int len){
  3. char a[300] = {0};
  4. if (len > 100) {
  5. memcpy(a, sz, 100);
  6. }
  7. a[len] = '\0';
  8. return a; // WRONG
  9. }

而应当使用堆来传递非简单类型变量。

  1. // Good
  2. char* Foo(char* sz, int len) {
  3. char* a = new char[300];
  4. if (len > 100) {
  5. memcpy(a, sz, 100);
  6. }
  7. a[len] = '\0';
  8. return a; // OK
  9. }

对于 C++ 程序来说,强烈建议返回 stringvector 等类型,会让代码更加简单和安全。

关联漏洞:

高风险-内存破坏

2.5 【必须】有逻辑联系的数组必须仔细检查

例如下列程序将字符串转换为week day,但是两个数组并不一样长,导致程序可能会越界读一个int。

  1. // Bad
  2. int nWeekdays[] = {1, 2, 3, 4, 5, 6};
  3. const char* sWeekdays[] = {"Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"};
  4. for (int x = 0; x < ARRAY_SIZE(sWeekdays); x++) {
  5. if (strcmp(sWeekdays[x], input) == 0)
  6. return nWeekdays[x];
  7. }

应当确保有关联的nWeekdays和sWeekdays数据统一。

  1. // Good
  2. const int nWeekdays[] = {1, 2, 3, 4, 5, 6, 7};
  3. const char* sWeekdays[] = {"Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"};
  4. assert(ARRAY_SIZE(nWeekdays) == ARRAY_SIZE(sWeekdays));
  5. for (int x = 0; x < ARRAY_SIZE(sWeekdays); x++) {
  6. if (strcmp(sWeekdays[x], input) == 0) {
  7. return nWeekdays[x];
  8. }
  9. }

关联漏洞:

高风险-内存破坏

2.6 【必须】避免函数的声明和实现不同

在头文件、源代码、文档中列举的函数声明应当一致,不应当出现定义内容错位的情况。

错误:

foo.h

  1. int CalcArea(int width, int height);

foo.cc

  1. int CalcArea(int height, int width) { // Different from foo.h
  2. if (height > real_height) {
  3. return 0;
  4. }
  5. return height * width;
  6. }

正确: foo.h

  1. int CalcArea(int height, int width);

foo.cc

  1. int CalcArea (int height, int width) {
  2. if (height > real_height) {
  3. return 0;
  4. }
  5. return height * width;
  6. }

关联漏洞:

中风险-逻辑问题

2.7 【必须】检查复制粘贴的重复代码(相同代码通常代表错误)

当开发中遇到较长的句子时,如果你选择了复制粘贴语句,请记得检查每一行代码,不要出现上下两句一模一样的情况,这通常代表代码哪里出现了错误:

  1. // Bad
  2. void Foobar(SomeStruct& foobase, SomeStruct& foo1, SomeStruct& foo2) {
  3. foo1.bar = (foo1.bar & 0xffff) | (foobase.base & 0xffff0000);
  4. foo1.bar = (foo1.bar & 0xffff) | (foobase.base & 0xffff0000);
  5. }

如上例,通常可能是:

  1. // Good
  2. void Foobar(SomeStruct& foobase, SomeStruct& foo1, SomeStruct& foo2) {
  3. foo1.bar = (foo1.bar & 0xffff) | (foobase.base & 0xffff0000);
  4. foo2.bar = (foo2.bar & 0xffff) | (foobase.base & 0xffff0000);
  5. }

最好是把重复的代码片段提取成函数,如果函数比较短,可以考虑定义为 inline 函数,在减少冗余的同时也能确保不会影响性能。

关联漏洞:

中风险-逻辑问题

2.8 【必须】左右一致的重复判断/永远为真或假的判断(通常代表错误)

这通常是由于自动完成或例如Visual Assistant X之类的补全插件导致的问题。

  1. // Bad
  2. if (foo1.bar == foo1.bar) {
  3. }

可能是:

  1. // Good
  2. if (foo1.bar == foo2.bar) {
  3. }

关联漏洞:

中风险-逻辑问题

2.9 【必须】函数每个分支都应有返回值

函数的每个分支都应该有返回值,否则如果函数走到无返回值的分支,其结果是未知的。

  1. // Bad
  2. int Foo(int bar) {
  3. if (bar > 100) {
  4. return 10;
  5. } else if (bar > 10) {
  6. return 1;
  7. }
  8. }

上述例子当bar<10时,其结果是未知的值。

  1. // Good
  2. int Foo(int bar) {
  3. if (bar > 100) {
  4. return 10;
  5. } else if (bar > 10) {
  6. return 1;
  7. }
  8. return 0;
  9. }

开启适当级别的警告(GCC 中为 -Wreturn-type 并已包含在 -Wall 中)并设置为错误,可以在编译阶段发现这类错误。

关联漏洞:

中风险-逻辑问题

中风险-信息泄漏

2.10 【必须】不得使用栈上未初始化的变量

在栈上声明的变量要注意是否在使用它之前已经初始化了

  1. // Bad
  2. void Foo() {
  3. int foo;
  4. if (Bar()) {
  5. foo = 1;
  6. }
  7. Foobar(foo); // foo可能没有初始化
  8. }

最好在声明的时候就立刻初始化变量,或者确保每个分支都初始化它。开启相应的编译器警告(GCC 中为 -Wuninitialized),并把设置为错误级别,可以在编译阶段发现这类错误。

  1. // Good
  2. void Foo() {
  3. int foo = 0;
  4. if (Bar()) {
  5. foo = 1;
  6. }
  7. Foobar(foo);
  8. }

关联漏洞:

中风险-逻辑问题

中风险-信息泄漏

2.11 【建议】不得直接使用刚分配的未初始化的内存(如realloc)

一些刚申请的内存通常是直接从堆上分配的,可能包含有旧数据的,直接使用它们而不初始化,可能会导致安全问题。例如,CVE-2019-13751。应确保初始化变量,或者确保未初始化的值不会泄露给用户。

  1. // Bad
  2. char* Foo() {
  3. char* a = new char[100];
  4. a[99] = '\0';
  5. memcpy(a, "char", 4);
  6. return a;
  7. }
  1. // Good
  2. char* Foo() {
  3. char* a = new char[100];
  4. memcpy(a, "char", 4);
  5. a[4] = '\0';
  6. return a;
  7. }

在 C++ 中,再次强烈推荐用 stringvector 代替手动内存分配。

关联漏洞:

中风险-逻辑问题

中风险-信息泄漏

2.12 【必须】校验内存相关函数的返回值

与内存分配相关的函数需要检查其返回值是否正确,以防导致程序崩溃或逻辑错误。

  1. // Bad
  2. void Foo() {
  3. char* bar = mmap(0, 0x800000, .....);
  4. *(bar + 0x400000) = '\x88'; // Wrong
  5. }

如上例mmap如果失败,bar的值将是0xffffffff (ffffffff),第二行将会往0x3ffffff写入字符,导致越界写。

  1. // Good
  2. void Foo() {
  3. char* bar = mmap(0, 0x800000, .....);
  4. if(bar == MAP_FAILED) {
  5. return;
  6. }
  7. *(bar + 0x400000) = '\x88';
  8. }

关联漏洞:

中风险-逻辑问题

高风险-越界操作

2.13 【必须】不要在if里面赋值

if里赋值通常代表代码存在错误。

  1. // Bad
  2. void Foo() {
  3. if (bar = 0x99) ...
  4. }

通常应该是:

  1. // Good
  2. void Foo() {
  3. if (bar == 0x99) ...
  4. }

建议在构建系统中开启足够的编译器警告(GCC 中为 -Wparentheses 并已包含在 -Wall 中),并把该警告设置为错误。

关联漏洞:

中风险-逻辑问题

2.14 【建议】确认if里面的按位操作

if里,非bool类型和非bool类型的按位操作可能代表代码存在错误。

  1. // Bad
  2. void Foo() {
  3. int bar = 0x1; // binary 01
  4. int foobar = 0x2; // binary 10
  5. if (foobar & bar) // result = 00, false
  6. ...
  7. }

上述代码可能应该是:

  1. // Good
  2. void foo() {
  3. int bar = 0x1;
  4. int foobar = 0x2;
  5. if (foobar && bar) // result : true
  6. ...
  7. }

关联漏洞:

中风险-逻辑问题