github security在推上发起了一个话题,经常会推送一个代码片段,让我们寻找其中的bug。

下面的代码是3.20的一个挑战。看不到图片的可以直接看下面的代码(为了方便阅读,我去掉了debug代码)

请输入图片描述

        u_int16_t s_offset = offset+extension_offset;
        u_int8_t version_len = packet->payload[s_offset];
        char version_str[256];
        u_int8_t version_str_len = 0;
        
        if(version_len == (extension_len-1)) {
          u_int8_t j;

          s_offset++;
          
          for(j=0; j<version_len; j += 2) {
            u_int16_t tls_version = ntohs(*((u_int16_t*)&packet->payload[s_offset+j]));
            u_int8_t unknown_tls_version;
            
            if((version_str_len+8) < sizeof(version_str)) {
              int rc = snprintf(&version_str[version_str_len],
                    sizeof(version_str) - version_str_len, "%s%s",
                    (version_str_len > 0) ? "," : "",
                    ndpi_ssl_version2str(tls_version, &unknown_tls_version));
              if(rc <= 0)
            break;
              else
            version_str_len += rc;
            }

结合代码中部分变量名来看的话,这里用改于ssl数据包的处理有关(但是我对这个协议不熟,没搞懂这里处理的目的是啥,只能瞎审)。

  • 前4行定义了一些相关变量,变量的类型是需要多注意一下的
  • 然后进入一条if语句判断
  • 定义了一个局部变量 u_int8_t j
  • s_offset ++ 下面可能要处理version_len字段之后的内容
  • for(j=0; j<version_len; j += 2) :j跟version_len同类型,都属于u_int8_t 因此它们的最大值都是255,但是j的初值=0,每次都会加2,因此j的值一定是“偶数值”,当j循环到254之后,还是会满足判断,但是j+=2会导致整数溢出,j变为0。所以当version_len=255的时候,会发生死循环

漏洞部分代码拿出来

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
int main(){
    uint8_t version_len = 255;
    uint8_t j = 0;
    int i = 0;
    for(j = 0; j<  version_len ;j += 2){
        printf("i : %d\tj : %d\n",i++,j);
    }
    return 0;
}

部分运行结果

i : 430074    j : 244
i : 430075    j : 246
i : 430076    j : 248
i : 430077    j : 250
i : 430078    j : 252
i : 430079    j : 254
i : 430080    j : 0
i : 430081    j : 2
i : 430082    j : 4
i : 430083    j : 6
i : 430084    j : 8

补丁:https://github.com/ntop/nDPI/commit/7806eb5f5b02fd78de1db20caeebc56088ebec3e

第一眼看到代码的时候,我以为问题在这个地方if((version_str_len+8) < sizeof(version_str)),虽然version_len是u_int16_t的类型,但是version_len+8之后的结果就不一定是u_int16_t了,因此这里不会发生整数溢出。

总结:在审计代码的时候,循环这种地方需要考虑一下边界条件,除了for循环还有while循环的退出条件。

preView