当前位置: 首页 > 百家 > 正文

给比特币做一次体检

不要指望这是一篇多么史诗的文章,这里只是用PVS-Studio检查了下比特币项目的源代码,并发现了其中一些可疑的片段。我想已经有很多程序员已经检查过比特币源代码了吧,不过既然我们也做了一次检查,这里就简单地说明下。

给比特币做一次体检

首先我们决定用PVS-Studio以及Clang进行对比测试,这是一个庞大而复杂的任务,我并不认为它能够很快完成。下面是这项任务中遇到的几个难题:

我们需要收集的项目是由GCC建立,但也可以由Clang进行编译。如果我们直接用Clang检查这些项目,Clang是不会发现漏洞的,因为在Clang的帮助下这些漏洞已经被修复了。但是PVS-Studio就可以。

一般来说,几乎没有项目可以同时在Clang以及Visual Studio上进行编译,Clang号称能够很好地兼容Visual Studio,但实际操作中却是另一码事,很多项目都不能正常建立和检查。而PVS-Studio在Linux平台下运行的情况却又是不如人意,因此我们所寻找的项目需要在这两种工具上都可以很好的进行处理。

比特币就是我们所选择的对比项目之一,两种工具运行完后所发现的错误也几乎为零,这也无需奇怪 —— 我想这个项目早就被检查烂了,这也就是为什么我们将大部分比较从中排除的原因,下面我们就简单地概括下这次检查。

项目分析

我想比特币是什么这里也就没必要进行介绍了,都快被玩坏了,源代码从这里下载: https://github.com/bitcoin/bitcoin

本次分析通过5.17版本PVS-Studio完成。

奇怪的循环

下面是该检测分析唯一我觉得可疑的片段。这是一段和密码学相关的函数,我不知道它到底有何作用,或许我找到了惊天秘密也说不定呢,现在这个年代,程序猿最乐意做的事就是发现一些关于安全性的大BUG,你懂的。但是,这里可能只是一个小错误,甚至是一段故意编写的代码。

bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn)  {   {    LOCK(cs_KeyStore);    if (!SetCrypted())    return false;      CryptedKeyMap::const_iterator mi = mapCryptedKeys.begin();    for (; mi != mapCryptedKeys.end(); ++mi)    {      const CPubKey &vchPubKey = (*mi).second.first;      const std::vector<unsigned char> &vchCryptedSecret =      (*mi).second.second;      CKeyingMaterial vchSecret;      if(!DecryptSecret(vMasterKeyIn, vchCryptedSecret,                      vchPubKey.GetHash(), vchSecret))        return false;      if (vchSecret.size() != 32)        return false;      CKey key;      key.Set(vchSecret.begin(), vchSecret.end(),            vchPubKey.IsCompressed());      if (key.GetPubKey() == vchPubKey)        break;      return false;    }    vMasterKey = vMasterKeyIn;  }  NotifyStatusChanged(this);  return true;  }  

注意这个循环,它必须遍历某些键值,然而,其循环体只执行一次。还有就是在该循环结束时,会有一个“return false;”操作,它也可以由“break;”终止,与此同时,并不存在一个单一的“continue;”的操作。

可疑的转变

static int64_t set_vch(const std::vector<unsigned char>& vch)  {    if (vch.empty())      return 0;      int64_t result = 0;    for (size_t i = 0; i != vch.size(); ++i)        result |= static_cast<int64_t>(vch[i]) << 8*i;      // If the input vector's most significant byte is 0x80,    // remove it from the result's msb and return a negative.    if (vch.back() & 0x80)        return -(result & ~(0x80 << (8 * (vch.size() - 1))));       return result;   }  

PVS-Studio的诊断信息:V629 检查了 ’0×80 << (8 * (vch.size() – 1))’语句,Bit从32位值扩展到64位值, script.h 169。

该功能形成了一个64位的值,其中一个转变是正确的,其他的可能不是。

正确的行:

 static_cast<int64_t>(vch[i]) << 8*i  

变量首先延伸到int64_t ,然后进行转移

可疑行:

 0x80 << (8 * (vch.size() - 1))  

该0×80的常数是’整数’类型,这意味着移动它可能会导致溢出。由于该函数生成了一个64位的值,我怀疑这里是有一个错误的。如果要了解更多关于准换的信息,可以参见,“Wade not in unknown waters – part three”这篇文章。

不变的代码:

 0x80ull << (8 * (vch.size() - 1))  

危险类

class CKey {    ....    // Copy constructor. This is necessary because of memlocking.    CKey(const CKey &secret) : fValid(secret.fValid),                           fCompressed(secret.fCompressed) {      LockObject(vch);      memcpy(vch, secret.vch, sizeof(vch));    }    ....  };  

PVS-Studio的诊断信息:V690中的’CKey’类实现了一个拷贝构造函数,但它缺少“=”操作符。使用这样的类是危险的。key.h 175

很多人认为拷贝构造函数是对于同步来说必须的,然而,一个项目不应该只通过拷贝构造函数来进行复制,“=”操作在这段代码中并未出现,尽管目前看来=操作还未能派上用场,但是这段代码还是有潜在危险的。

同样的,还有一些需要进行仔细检查的类:

V690 “Semantic_actions”类实现了’=’操作,但缺少一个拷贝构造函数,使用这样的类是危险的。json_spirit_reader_template.h196

V690 “CFeeRate”类实现一个拷贝构造函数,但缺少“=”操作符,使用这样的类是危险的。 core.h118

V690 “CTransaction”类实现了’=’操作,但缺少一个拷贝构造函数,使用这样的类是危险的。core.h212

V690 “CTxMemPoolEntry”类实现一个拷贝构造函数,但缺少“=”操作符,使用这样的类是危险的。txmempool.h27

V690 “Json_grammer”类实现了’=’操作,但缺少一个拷贝构造函数,使用这样的类是危险的。 json_spirit_reader_template.h370

V690 ‘Generator’类实现了’=’操作,但缺少一个拷贝构造函数,使用这样的类是危险的。 json_spirit_writer_template.h98

结论

定期使用静态分析器可以帮助你节省大量的时间和神经细胞,最主要的是,它很方便,哈哈,比特币君挺住啊,别被玩坏咯。

 

原文:http://www.viva64.com/en/b/0268/?utm_source=bitcoinweekly&utm_medium=email#ID0EKWBI

作者:Andrey Karpov

翻译:小蒙牛

转自:巴比特

固定链接: 给比特币做一次体检 | 三个硬币

【上一篇】
【下一篇】

该日志由 bitman 于2014年08月07日发表在 百家 分类下, 你可以发表评论,并在保留原文地址及作者的情况下引用到你的网站或博客。
原创文章转载请注明: 给比特币做一次体检 | 三个硬币
关键字:

给比特币做一次体检:等您坐沙发呢!

发表评论

快捷键:Ctrl+Enter