CodeQL之CWE-252

CWE-252: Unchecked Return Value

The software does not check the return value from a method or function, which can prevent it from detecting unexpected states and conditions.

顾名思义，Unchecked Return Value，就是在实际代码中缺少了对返回值的检查，在实际代码中，对错误返回值的检查尤为重要，因为需要进入到相应的错误处理环节中去，否则会影响程序的正常运行。比如我们常见的对malloc返回值的检查等等，和CWE-252相关的还有CWE-754、CWE-273、CWE-476

PS：我们以openssl为例进行相应的CodeQL查询脚本的编写

我们要找的是缺少对函数返回值的检查的情况，但首先我们要知道哪些函数的返回值是需要进行检查的。

可以想到的有以下两种方法：

• 思路1：我们可以从文档的描述里知道某个函数的返回值是需要进行检查的，例如openssl文档中，我们通过下面的描述知道RSA_new在返回NULL的时候是出现了错误，因此需要进行检查

  If the allocation fails, RSA_new() returns NULL and sets an error code that can be obtained by ERR_get_error(3). Otherwise it returns a pointer to the newly allocated structure.

• 思路2：当一个函数，在项目中被多次使用，大部分都检查了返回值，那么这个函数的返回值大概率是要被检查的，而没检查的地方就是Unchecked Return Value

以RSA_new为例，我们先看看它有没有忘检查返回值的情况

import cpp
predicate check_name(Function func) { func.getName().matches("RSA_new") }

from FunctionCall func
where check_name(func.getTarget())
select func,func.getLocation()

可以看到有17个地方使用了RSA_new，其中16个地方，有的直接用IF判断是否为NULL，有的使用TEST_ptr判断是否是NULL，只有最后一处，在openssl的测试代码里，忘记了检查

static int rsa_setkey(RSA** key, unsigned char* ctext, int idx)
{
    int clen = 0;
    *key = RSA_new();
    switch (idx) {
    case 0:
        clen = key1(*key, ctext);
        break;
    case 1:
        clen = key2(*key, ctext);
        break;
    case 2:
        clen = key3(*key, ctext);
        break;
    }
    return clen;
}

由于在测试代码里，所以问题不大，不过还是提了个issue，开发者也很快补上了(https://github.com/openssl/openssl/issues/13361)

思路1#

思路1：从文档的描述里知道某个函数的返回值是需要进行检查的，例如openssl文档中，通过下面的描述知道RSA_new在返回NULL的时候是出现了错误，因此需要进行检查

我们前面已知对RSA_new的返回值需要进行检查，虽然直接找就找到了17个，看起来也很快，但是当函数很多，结果很多的时候，就有点浪费时间了，我们最好是让查询脚本只找到最后一个结果，也就是降低误报率

简单的看下17个，可以发现大部分都是一个变量接住返回值，然后进行检查，因此我们可以很快的写出下面的查询脚本

import cpp
import semmle.code.cpp.controlflow.Guards
import semmle.code.cpp.dataflow.DataFlow

predicate check_name(Function func) { func.getName().matches("RSA_new") }

from FunctionCall func, GuardCondition cond
where
  check_name(func.getTarget()) and
  exists(DataFlow::Node source, DataFlow::Node sink, int diff |
    DataFlow::localFlow(source, sink) and
    source.asExpr().(FunctionCall) = func and
    cond.getAChild*() = sink.asExpr() and
    diff = sink.getLocation().getEndLine() - source.getLocation().getEndLine() and
    diff >= 0 and
    diff < 10
  )
select func,func.getLocation()

本来我们应该要查not exists的，但是由于查not exsits的搜索空间更大，耗时间也耗计算资源，所以我们直接差exsits，然后和一开始的17个结果区差集，就是not exsits的结果。

查询脚本的内容很简单，首先还是找函数调用，检查是不是RSA_new这个函数，然后找是不是存在函数内数据流，source点是函数调用，sink点是变量的使用点，这个变量的使用点还必须是在一个条件语句中的，最后简单的通过行数差来看是不是一个检查点(因为一般check点都不会很远)。更加合理的方法是写成找最近的条件语句sink点，这样我们查询到的exsits结果有14个，如下

not exsits的3个中，两个误报的情况分别如下

// case 1
loopargs[i].rsa_key[testnum] = RSA_new();
if (loopargs[i].rsa_key[testnum] == NULL) {
     BN_free(bn);
     goto end;
}

//case 2
*pval = (ASN1_VALUE *)RSA_new();
if (*pval != NULL)
    return 2;

问题的原因在于codeql没有提取到if中对变量的使用这个sink点(通过去掉判断打印出sink点可以发现，不过今天下载了最新的codeql重新编译了数据库，case2已经可以识别出来了)，如果大家感兴趣，可以继续修改查询语句看怎么去除这两个误报，也欢迎一起讨论解决。

思路2#

当一个函数，在项目中被多次使用，大部分都检查了返回值，那么这个函数的返回值大概率是要被检查的，而没检查的地方就是Unchecked Return Value

import cpp
import semmle.code.cpp.controlflow.Guards
import semmle.code.cpp.dataflow.DataFlow

predicate check_name(Function func) { func.getName().matches("RSA_new") }

from FunctionCall func, int total, int checked
where
  check_name(func.getTarget()) and
  total = count(FunctionCall fc | check_name(fc.getTarget())) and
  checked =
    count(FunctionCall fc |
      check_name(fc.getTarget()) and
      exists(DataFlow::Node source, DataFlow::Node sink, int diff, GuardCondition cond |
        DataFlow::localFlow(source, sink) and
        source.asExpr().(FunctionCall) = fc and
        cond.getAChild*() = sink.asExpr() and
        diff = sink.getLocation().getEndLine() - source.getLocation().getEndLine() and
        diff >= 0 and
        diff < 10
      )
    ) and
  checked != total and
  checked * 100 / total >= 80 and
  exists(DataFlow::Node source, DataFlow::Node sink, int diff, GuardCondition cond |
    DataFlow::localFlow(source, sink) and
    source.asExpr().(FunctionCall) = func and
    cond.getAChild*() = sink.asExpr() and
    diff = sink.getLocation().getEndLine() - source.getLocation().getEndLine() and
    diff >= 0 and
    diff < 10
  )
select func, func.getLocation()

在思路2的查询脚本中，我们先是统计了全局使用了RSA_new几次(total=...)，然后统计了有几次使用后进行了检查(checked=...)，然后设置了阈值为80%，最后，当本身未进行检查时，将结果给出，同样这里还是选择用exsits取差集代替not exsits，结果如下

可以看到依旧是找到了14个存在检查的，差集中存在2个误报。

总结#

不管是思路1还是思路2，其实在自动化上都存在一定困难。

1. 对于思路1，难点在于没有好的方法去知道某API需要进行返回值检查，有的选择正则表达式在文档里找，也有选择用NLP方法去从文档里提取信息，但仍然有很长的路要走，是个难点，而且有文档的项目毕竟还是少数
2. 对于思路2，阈值的设置是个问题，和项目中API的使用数量等挂钩，而且并不能保证不检查就一定有问题，只是大概率有问题而已

参考资料：

• CWE-252

• CodeQL