A.
不使用编程方法和工具来帮助控制代码质量,就不可能开发大型复杂项目。 首先,它是一个合格的编码标准,代码审查,单元测试,静态和动态代码分析器。 所有这些都有助于在开发的最早阶段识别代码中的缺陷。 本文演示了PVS-Studio静态分析器检测Android操作系统代码中的错误和潜在漏洞的功能。 我们希望本文能够吸引读者使用静态代码分析的方法,并且希望在开发自己的项目的过程中实现它。
引言
自从撰写有关Tizen操作系统错误的大型
文章以来已经过去了一年,我想再次对某些操作系统进行同样有趣的研究。 选择落在Android上。
Android操作系统代码质量良好,经过了良好的测试。 在开发过程中,至少使用Coverity静态代码分析器,如以下形式的注释所示:
通常,这是一个有趣的高质量项目,对于我们的PVS-Studio静态分析仪而言,发现错误是一项挑战。
我认为,仅从文章的数量上,读者就可以了解PVS-Studio分析仪的出色工作,并发现该操作系统代码中的许多缺陷。
常见弱点列举
在本文中,您将找到对
常见弱点枚举 (CWE)的引用。 让我们从安全的角度解释引用此列表的原因以及为什么它很重要。
通常,程序漏洞的原因不是某些棘手的情况,而是常见的软件错误。 在这里引用
prqa.com的报价是适当的:
美国国家标准技术研究院(NIST)报告说,64%的软件漏洞源于编程错误,而不是缺乏安全功能。
您可以阅读文章“
PVS-Studio如何帮助发现漏洞? ”,其中包含一些简单错误的示例,这些错误导致了MySQL,iOS,NAS,illumos-gate等项目中的漏洞。
因此,可以通过及时检测并修复常见错误来消除许多漏洞。 在这里,普通弱点枚举进入了场景。
错误是不同的,并且从安全角度来看,并非所有错误都是危险的。 常见弱点枚举中收集了可能导致漏洞的错误。 此列表正在更新中,可能存在可能导致漏洞的错误,但尚未包含在此列表中。
但是,如果错误是根据CWE分类的,则从理论上讲它有可能被利用为漏洞(
CVE )。 是的,这种可能性很小。 CWE很少变成CVE。 但是,如果您想保护代码免受漏洞侵害,则应尽可能找到CWE中描述的错误并加以修复。
示意图中,PVS-Studio,错误,CWE和CVE之间的关系如图所示:
一些错误被分类为CWE。 PVS-Studio可以检测到许多此类错误,从而防止其中一些缺陷成为漏洞(CVE)。
可以说,PVS-Studio在潜在危害造成危害之前就已将其识别出来。 因此,PVS-Studio是静态应用程序安全测试工具(
SAST )。
现在,我认为,很清楚为什么在描述错误时,我认为重要的是要注意根据CWE对错误进行分类。 这样一来,很容易显示在操作系统与之明显相关的关键项目中使用静态代码分析器的重要性。
Android检查
为了分析该项目,我使用了PVS-Studio分析仪版本6.24。 该分析器当前支持以下语言和编译器:
- 窗户 Visual Studio 2010-2017 C,C ++,C ++ / CLI,C ++ / CX(WinRT),C#
- 窗户 IAR嵌入式工作台,用于ARM C,C ++的C / C ++编译器
- Windows / Linux Keil µVision,DS-MDK,ARM编译器5/6 C,C ++
- Windows / Linux 德州仪器Code Composer Studio,ARM代码生成工具C,C ++
- Windows / Linux / macOS。 Clang C,C ++
- Linux / macOS。 GCC C,C ++
- 窗户 MinGW C,C ++
注意事项 也许有些读者错过了我们支持macOS环境下工作的消息,他们会对本出版物感兴趣:“
针对MacOS的PVS-Studio版本:Apple XNU Kernel的64个弱点 ”。
检查Android源代码的过程不是问题,因此我不再赘述。 相反,问题是我全神贯注于其他任务,这就是为什么我没有找到时间和精力来尽我所能仔细地查看报告。 但是,即使快速浏览,也不足以为可靠的文章收集大量有趣的错误。
最重要的是:我要求Android开发人员不仅要纠正文章中描述的错误,还要进行更仔细的独立分析。 我从表面上看了分析仪报告,可能错过了很多严重的错误。
在第一次测试时,分析仪会产生很多误报,但这
不是问题 。 我们的团队随时准备提供有关配置分析仪以减少误报次数的建议。 如果需要,我们还准备提供一个月或更长时间的许可证密钥。 通常,
给我们写信 ,我们将帮助您。
现在,让我们看看我设法找到了哪些错误和潜在漏洞。 希望您喜欢PVS-Studio静态代码分析器可以找到的东西。 阅读愉快。
无意义的比较
如果表达式始终为true或false,则分析器认为它们为异常。 根据常见弱点枚举,此类警告分类为:
分析器会生成许多这样的警告,但不幸的是,大多数警告对于Android代码都是错误的。 在这种情况下,不要责怪分析仪。 只是代码是这样写的。
我将通过一个简单的示例对此进行演示。
#if GENERIC_TARGET const char alternative_config_path[] = "/data/nfc/"; #else const char alternative_config_path[] = ""; #endif CNxpNfcConfig& CNxpNfcConfig::GetInstance() { .... if (alternative_config_path[0] != '\0') { .... }
分析器在此处生成警告:V547 CWE-570表达式'alternative_config_path [0]!='\ 0''始终为false。 phNxpConfig.cpp 401
事实是未定义
GENERIC_TARGET宏,并且从分析器的角度来看,代码如下所示:
const char alternative_config_path[] = ""; .... if (alternative_config_path[0] != '\0') {
由于该行为空,并且端子零始终位于零偏移处,因此分析仪只需发出警告。 因此,分析仪在发出警告时是正确的。 但是,从实际的角度来看,此警告没有任何好处。
不幸的是,在这种情况下什么也做不了。 我们将必须系统地审查所有此类警告,并将许多地方标记为误报,以使分析仪不再在这些行上发出消息。 确实需要这样做,因为除了无意义的消息外,还会发现很多实际的缺陷。
我坦诚地承认,我对仔细查看这种类型的警告不感兴趣,因此我对这些警告进行了简要介绍。 但是,即使这样也足以表明此类诊断非常有用并找到有趣的错误。
我想从经典情况开始,即比较两个对象的功能未正确实现。 为什么要经典? 这是我们在各种项目中经常遇到的典型错误模式。 最可能的原因有以下三个:
- 比较功能很简单,并使用“复制粘贴”技术“自动”编写。 当编写这样的代码时,一个人会不专心,经常打错字。
- 通常,对于此类功能不执行代码审查,因为它太懒了,无法查看简单而乏味的功能。
- 通常不对此类功能进行单元测试。 因为懒惰。 另外,这些函数很简单,并且程序员认为它们中可能没有错误。
这些想法和示例在“
邪恶生活在比较功能中 ”一文中有更详细的描述。
static inline bool isAudioPlaybackRateEqual( const AudioPlaybackRate &pr1, const AudioPlaybackRate &pr2) { return fabs(pr1.mSpeed - pr2.mSpeed) < AUDIO_TIMESTRETCH_SPEED_MIN_DELTA && fabs(pr1.mPitch - pr2.mPitch) < AUDIO_TIMESTRETCH_PITCH_MIN_DELTA && pr2.mStretchMode == pr2.mStretchMode && pr2.mFallbackMode == pr2.mFallbackMode; }
因此,这是比较两个
AudioPlaybackRate类型的对象的经典函数。 而且,正如我认为的那样,读者猜测这是错误的。 PVS-Studio分析仪会立即注意到两种错别字:
- V501 CWE-571“ ==”运算符的左侧和右侧有相同的子表达式:pr2.mStretchMode == pr2.mStretchMode AudioResamplerPublic.h 107
- V501 CWE-571“ ==”运算符的左侧和右侧有相同的子表达式:pr2.mFallbackMode == pr2.mFallbackMode AudioResamplerPublic.h 108
将
pr2.mStretchMode字段和
pr2.mFallbackMode字段
进行比较。 事实证明,该功能不能足够准确地比较对象。
以下无意义的比较存在于将指纹信息存储在文件中的函数中。
static void saveFingerprint(worker_thread_t* listener, int idx) { .... int ns = fwrite(&listener->secureid[idx], sizeof(uint64_t), 1, fp); .... int nf = fwrite(&listener->fingerid[idx], sizeof(uint64_t), 1, fp); if (ns != 1 || ns !=1)
一次通过两个诊断程序在此代码中检测到异常:
- V501 CWE-570'||'的左侧和右侧有相同的子表达式 运算符:ns!= 1 || ns!= 1个指纹.c 126
- V560 CWE-570条件表达式的一部分始终为false:ns!= 1. Fingerprint.c 126
当第二次调用
fwrite函数无法将数据写入文件时,将无法处理这种情况。 换句话说,不检查变量
nf的值。 正确的检查应如下所示:
if (ns != 1 || nf != 1)
我们继续处理与
&运算符相关的下一个错误。
#define O_RDONLY 00000000 #define O_WRONLY 00000001 #define O_RDWR 00000002 static ssize_t verity_read(fec_handle *f, ....) { .... if (f->mode & O_RDONLY && expect_zeros) { memset(data, 0, FEC_BLOCKSIZE); goto valid; } .... }
PVS-Studio警告:V560 CWE-570条件表达式的一部分始终为false:f-> mode&00000000。fec_read.cpp 322
注意常数
O_RDONLY为零。 这使表达式
f-> mode&O_RDONLY毫无意义,因为它始终为0。事实证明,
if语句的条件从不满足,而statement-true是无效代码。
正确的检查应如下所示:
if (f->mode == O_RDONLY && expect_zeros) {
现在让我们看一下当我们忘记写条件的一部分时的经典错字。
enum { .... CHANGE_DISPLAY_INFO = 1 << 2, .... }; void RotaryEncoderInputMapper::configure(nsecs_t when, const InputReaderConfiguration* config, uint32_t changes) { .... if (!changes || (InputReaderConfiguration::CHANGE_DISPLAY_INFO)) { .... }
PVS-Studio警告:V768 CWE-571枚举常量'CHANGE_DISPLAY_INFO'用作布尔型变量。 InputReader.cpp 3016
该条件始终为true,因为操作数
InputReaderConfiguration :: CHANGE_DISPLAY_INFO是等于4的常数。
如果您查看附近的代码编写方式,那么很明显,实际上条件应该是这样的:
if (!changes || (changes & InputReaderConfiguration::CHANGE_DISPLAY_INFO)) {
下面的比较是没有意义的,我在循环运算符中遇到了。
void parse_printerAttributes(....) { .... ipp_t *collection = ippGetCollection(attrptr, i); for (j = 0, attrptr = ippFirstAttribute(collection); (j < 4) && (attrptr != NULL); attrptr = ippNextAttribute(collection)) { if (strcmp("....", ippGetName(attrptr)) == 0) { ....TopMargin = ippGetInteger(attrptr, 0); } else if (strcmp("....", ippGetName(attrptr)) == 0) { ....BottomMargin = ippGetInteger(attrptr, 0); } else if (strcmp("....", ippGetName(attrptr)) == 0) { ....LeftMargin = ippGetInteger(attrptr, 0); } else if (strcmp("....", ippGetName(attrptr)) == 0) { ....RightMargin = ippGetInteger(attrptr, 0); } } .... }
PVS-Studio警告:V560 CWE-571条件表达式的一部分始终为true:(j <4)。 ipphelper.c 926
请注意,变量
j的值不会在任何地方递增。 这意味着子表达式
(j <4)始终为true。
涉及真/假条件的PVS-Studio分析仪最大数量的有用操作,是指使用
new运算符检查对象创建结果的代码。 换句话说,分析器检测以下代码模式:
T *p = new T; if (p == nullptr) return ERROR;
这样的检查是没有意义的。 如果无法为该对象分配内存,则会抛出
std :: bad_alloc类型的
异常,并且该
异常将无法检查指针值。
注意事项
new运算符可以
通过写入
new(std :: nothrow)T来返回
nullptr 。 但是,这不适用于所讨论的错误。 如果以这种方式创建对象,PVS-Studio分析仪会考虑
(std :: nothrow) ,并且不会给出警告。
此类错误似乎无害。 好吧,想想看,一项额外的检查永远都行不通。 无论如何,将引发异常,该异常将在某处进行处理。 不幸的是,一些开发人员将
if语句操作释放为true,以释放资源等。 由于此代码未执行,因此可能导致内存泄漏和其他错误。
考虑一下我在Android代码中注意到的这些情况之一。
int parse_apk(const char *path, const char *target_package_name) { .... FileMap *dataMap = zip->createEntryFileMap(entry); if (dataMap == NULL) { ALOGW("%s: failed to create FileMap\n", __FUNCTION__); return -1; } char *buf = new char[uncompLen]; if (NULL == buf) { ALOGW("%s: failed to allocate %" PRIu32 " byte\n", __FUNCTION__, uncompLen); delete dataMap; return -1; } .... }
PVS-Studio警告:V668 CWE-570测试“ buf”指针是否为空没有意义,因为使用“ new”运算符分配了内存。 如果内存分配错误,将生成异常。 第213章
请注意,如果不可能分配第二个内存块,程序员将尝试释放第一个内存块:
delete dataMap;
此代码将永远无法控制。 这是无效代码。 如果发生异常,则会发生内存泄漏。
编写这样的代码从根本上是错误的。 在这种情况下发明了
智能指针 。
在使用
new创建对象之后,PVS-Studio分析器总共在Android代码中检测到
176个指针检查位置。 我不了解这些地方的危险性,当然,我不会在所有这些警告中弄乱文章。 有
兴趣的人可以在
Android_V668.txt文件中看到其他警告。
解引用空指针
取消引用空指针会导致程序行为未定义,因此查找并修复此类位置非常有用。 根据情况,PVS-Studio分析仪可以根据常见弱点枚举对这些错误进行分类,如下所示:
我经常在负责处理非标准或不正确情况的代码中发现此类错误。 没有人会测试这样的代码,并且该错误会持续很长时间。 现在将考虑这种情况。
bool parseEffect(....) { .... if (xmlProxyLib == nullptr) { ALOGE("effectProxy must contain a <%s>: %s", tag, dump(*xmlProxyLib)); return false; } .... }
PVS-Studio警告:V522 CWE-476可能会取消引用空指针“ xmlProxyLib”。 EffectsConfig.cpp 205
如果
xmlProxyLib指针为
nullptr ,则程序员将显示一条调试消息,为此必须取消引用该指针。 糟糕...
现在考虑该错误的更有趣的版本。
static void soinfo_unload_impl(soinfo* root) { .... soinfo* needed = find_library(si->get_primary_namespace(), library_name, RTLD_NOLOAD, nullptr, nullptr); if (needed != nullptr) {
PVS-Studio警告:V522 CWE-476可能会取消引用“需要”空指针。 链接器.cpp 1847
如果
需要指针
!= Nullptr ,则会发出警告,这是程序的非常可疑的行为。 最终可以很清楚地看到,如果您看下面的代码,则该代码将包含一个错误,当
需要== nullptr时 ,将在表达式
needed-> is_linked()中取消引用空指针。
这里很可能会混淆运算符!=和==。 如果更换,则功能代码有意义,错误消失。
关于空指针可能被取消引用的大量警告都涉及以下形式的情况:
T *p = (T *) malloc (N); *p = x;
如果无法分配内存,则诸如
malloc ,
strdup等函数可以返回
NULL 。 因此,如果不先检查指针,就不能取消引用返回这些功能的指针。
有许多类似的错误,因此我只给出两个简单的代码片段:第一个包含
malloc和第二个与
strdup 。
DownmixerBufferProvider::DownmixerBufferProvider(....) { .... effect_param_t * const param = (effect_param_t *) malloc(downmixParamSize); param->psize = sizeof(downmix_params_t); .... }
PVS-Studio警告:V522 CWE-690可能会取消引用潜在的空指针“参数”。 检查行:245、244。BufferProviders.cpp 245
static char* descriptorClassToDot(const char* str) { .... newStr = strdup(lastSlash); newStr[strlen(lastSlash)-1] = '\0'; .... }
PVS-Studio警告:V522 CWE-690可能会取消引用潜在的空指针'newStr'。 检查行:203、202。DexDump.cpp 203
可能有人会说这些是小错误。 如果没有足够的内存,则在取消引用空指针时程序将简单崩溃,这是正常的。 由于没有内存,因此无需尝试以某种方式处理这种情况。
这样的人是错的。 指针必须检查! 我在“
为什么检查malloc函数返回什么很重要 ”一文中详细研究了此主题。 我强烈建议您将其阅读给所有未读过的人。
简而言之,危险是不一定要在零地址附近进行对存储器的写操作。 可能会将数据写到不受写保护的内存页中很远的某个地方,从而导致难以捉摸的错误,或者通常将此错误用作漏洞。 让我们看看
check_size函数示例的
含义 。
int check_size(radio_metadata_buffer_t **metadata_ptr, const uint32_t size_int) { .... metadata = realloc(metadata, new_size_int * sizeof(uint32_t)); memmove( (uint32_t *)metadata + new_size_int - (metadata->count + 1), (uint32_t *)metadata + metadata->size_int - (metadata->count + 1), (metadata->count + 1) * sizeof(uint32_t)); .... }
PVS-Studio警告:V769 CWE-119“(uint32_t *)元数据+ new_size_int”表达式中的“(uint32_t *)元数据”指针可能为nullptr。 在这种情况下,结果值将毫无意义,因此不应使用。 检查行:91,89。radio_metadata.c 91
我不了解该功能的逻辑,但这不是必需的。 最主要的是创建了一个新的缓冲区,并将数据复制到那里。 如果
realloc函数返回
NULL ,则数据将被复制到地址((uint32_t *)NULL +元数据-> size_int-(元数据->计数+1))。
如果
metadata-> size_int值很大,那么后果将令人遗憾。 事实证明,数据被写入到随机存储器中。
顺便说一句,还有另一种空指针取消引用,PVS-Studio分析器将其归类为CWE-628(无效参数)不是CWE-690。
static void parse_tcp_ports(const char *portstring, uint16_t *ports) { char *buffer; char *cp; buffer = strdup(portstring); if ((cp = strchr(buffer, ':')) == NULL) .... }
PVS-Studio警告:V575 CWE-628潜在的空指针传递到'strchr'函数中。 检查第一个参数。 检查行:47,46。libxt_tcp.c 47
事实是,
调用strchr函数时将发生指针取消引用。 因此,分析器将这种情况解释为将错误的值传递给函数。
我在
Android_V522_V575.txt文件中列出了其余
194条此类警告。
顺便说一下,前面讨论过的关于在调用
new运算符之后检查指针的警告给出了所有这些错误的特殊提示。 事实证明,当不检查指针时,有195个对
malloc /
realloc /
strdup函数的调用,依此类推。 但是在调用
new之后有176个地方检查指针。 同意,这很奇怪!
最后,我们还要考虑警告V595和V1004,它们也与使用空指针相关联。
V595检测到指针被取消引用然后检查的情况。 综合示例:
p->foo(); if (!p) Error();
当首先检查指针,然后忘记这样做时,V1004会显示相反的情况。 综合示例:
if (p) p->foo(); p->doo();
让我们看看一些Android代码片段,其中有这种类型的错误。
特别说明其功能不是必需的。 PV_STATUS RC_UpdateBuffer(VideoEncData *video, Int currLayer, Int num_skip) { rateControl *rc = video->rc[currLayer]; MultiPass *pMP = video->pMP[currLayer]; if (video == NULL || rc == NULL || pMP == NULL) return PV_FAIL; .... }
PVS-Studio警告:V595 CWE-476在针对nullptr进行验证之前,已使用了“视频”指针。检查行:385、388。rate_control.cpp 385 static void resampler_reset(struct resampler_itfe *resampler) { struct resampler *rsmp = (struct resampler *)resampler; rsmp->frames_in = 0; rsmp->frames_rq = 0; if (rsmp != NULL && rsmp->speex_resampler != NULL) { speex_resampler_reset_mem(rsmp->speex_resampler); } }
PVS-Studio警告:V595 CWE-476在针对nullptr进行验证之前,已使用了'rsmp'指针。检查线:54,57. resampler.c 54 void bta_gattc_disc_cmpl(tBTA_GATTC_CLCB* p_clcb, UNUSED_ATTR tBTA_GATTC_DATA* p_data) { .... if (p_clcb->status != GATT_SUCCESS) { if (p_clcb->p_srcb) { std::vector<tBTA_GATTC_SERVICE>().swap( p_clcb->p_srcb->srvc_cache); } bta_gattc_cache_reset(p_clcb->p_srcb->server_bda); } .... }
PVS-Studio警告:V1004 CWE-476在针对nullptr进行验证之后,不安全地使用了'p_clcb-> p_srcb'指针。检查行:695,701。bta_gattc_act.cc 701考虑这种类型的其他警告并不有趣。其中包括由于不良或困难的代码编写而引起的错误和错误警告。我写了许多有用的警告:- V1004 CWE-476 The 'ain' pointer was used unsafely after it was verified against nullptr. Check lines: 101, 105. rsCpuIntrinsicBLAS.cpp 105
- V595 CWE-476 The 'outError' pointer was utilized before it was verified against nullptr. Check lines: 437, 450. Command.cpp 437
- V595 CWE-476 The 'out_last_reference' pointer was utilized before it was verified against nullptr. Check lines: 432, 436. AssetManager2.cpp 432
- V595 CWE-476 The 'set' pointer was utilized before it was verified against nullptr. Check lines: 4524, 4529. ResourceTypes.cpp 4524
- V595 CWE-476 The 'reply' pointer was utilized before it was verified against nullptr. Check lines: 126, 133. Binder.cpp 126
- V595 CWE-476 The 'video' pointer was utilized before it was verified against nullptr. Check lines: 532, 540. rate_control.cpp 532
- V595 CWE-476 The 'video' pointer was utilized before it was verified against nullptr. Check lines: 702, 711. rate_control.cpp 702
- V595 CWE-476 The 'pInfo' pointer was utilized before it was verified against nullptr. Check lines: 251, 254. ResolveInfo.cpp 251
- V595 CWE-476 The 'address' pointer was utilized before it was verified against nullptr. Check lines: 53, 55. DeviceHalHidl.cpp 53
- V595 CWE-476 The 'halAddress' pointer was utilized before it was verified against nullptr. Check lines: 55, 82. DeviceHalHidl.cpp 55
然后我很无聊,并且过滤掉了此类警告。因此,我什至不知道分析仪检测到了多少实际错误。这些警告正在等待他们的英雄,他们将仔细研究它们并更改代码。我只想提请新读者注意这种错误: NJ_EXTERN NJ_INT16 njx_search_word(NJ_CLASS *iwnn, ....) { .... NJ_PREVIOUS_SELECTION_INFO *prev_info = &(iwnn->previous_selection); if (iwnn == NULL) { return NJ_SET_ERR_VAL(NJ_FUNC_NJ_SEARCH_WORD, NJ_ERR_PARAM_ENV_NULL); } .... }
PVS-Studio警告:V595 CWE-476在针对nullptr进行验证之前,已使用了'iwnn'指针。检查行:686,689。ndapi.c 686有些人认为这里没有错误,因为“没有真正的指针取消引用”。不存在的变量的地址可以简单地计算出来。此外,如果指针iwnn为零,则函数将退出。因此,没有发生什么不好的事情,我们先前错误地计算了类成员的地址。不,你不能那样说话。此代码导致未定义的行为,因此不能这样写。未定义的行为可以表现出来,例如,如下所示:- 编译器看到指针已取消引用:iwnn-> previous_selection
- 您不能取消引用空指针,因为这是未定义的行为
- 编译器得出的结论是,iwnn指针始终为非零
- 编译器删除了多余的检查:if(iwnn == NULL)
- 就这样,现在在执行程序时,不执行对空指针的检查,并且工作从指向类成员的不正确指针开始
我的文章“ 取消引用空指针会导致未定义的行为 ”中对此主题进行了详细描述。私有数据不会在内存中删除
考虑一种严重的潜在漏洞,该漏洞根据“常见漏洞枚举”分类为CWE-14:清除缓冲区的编译器删除代码。简而言之,这就是底线:如果在此之后不再使用缓冲区,则编译器有权删除memset函数调用。当我写这种类型的漏洞时,必然会出现这样的评论:这只是编译器中的一个小故障,需要修复。不,这不是小故障。在反对之前,请阅读以下材料:总的来说,一切都是认真的。Android中是否存在此类错误?当然有
它们通常有很多:证明 :)。让我们回到Android代码并考虑FwdLockGlue_InitializeRoundKeys函数的开头和结尾,因为中间的内容对我们而言并不有趣。 static void FwdLockGlue_InitializeRoundKeys() { unsigned char keyEncryptionKey[KEY_SIZE]; .... memset(keyEncryptionKey, 0, KEY_SIZE);
PVS-Studio警告:V597 CWE-14编译器可以删除“内存集”函数调用,该函数调用用于刷新“ keyEncryptionKey”缓冲区。 memset_s()函数应用于擦除私有数据。 FwdLockGlue.c 102 keyEncryptionKey数组在堆栈上创建,并存储私有信息。在函数的最后,他们想用零填充该数组,以免它意外地到达不应到达的位置。文章“ 覆盖内存-为什么? ” 将告诉您信息如何到达不应到达的位置。要使用零填充私有信息来填充数组,请使用memset函数。评论“归零关键数据”确认我们正确理解了所有内容。问题在于,编译器在构建发行版本时很有可能会删除memset函数调用。由于未使用memset调用之后的缓冲区,因此从编译器的角度来看,memset函数本身的调用是多余的。更多10个警告我写了一个文件Android_V597.txt。还有一个错误是内存没有被覆盖,尽管在这种情况下memset函数与此无关。 void SHA1Transform(uint32_t state[5], const uint8_t buffer[64]) { uint32_t a, b, c, d, e; .... a = b = c = d = e = 0; }
PVS-Studio警告:V1001 CWE-563分配了'a'变量,但直到功能结束后才使用。sha1.c 213PVS-Studio检测到与以下事实有关的异常:在将值分配给变量后,不再使用它们。分析仪将此缺陷分类为CWE-563:不使用变量分配。而且,从形式上来说,他是对的,尽管事实上,我们在这里再次处理CWE-14。编译器将丢弃这些分配,因为从C和C ++的角度来看,它们是多余的。结果,堆栈将保留存储私有数据的变量a,b,c,d和e的旧值。未指定/实施定义的行为
当您不累时,让我们看一个复杂的案例,这需要我本人进行详尽的描述。 typedef int32_t GGLfixed; GGLfixed gglFastDivx(GGLfixed n, GGLfixed d) { if ((d>>24) && ((d>>24)+1)) { n >>= 8; d >>= 8; } return gglMulx(n, gglRecip(d)); }
PVS-Studio警告:V793奇怪的是'(d >> 24)+ 1'语句的结果是条件的一部分。也许,该陈述应该与其他陈述进行比较。fixed.cpp 75程序员想要检查变量d的8个高阶位是否包含单位,但不是一次包含所有位。换句话说,程序员希望验证高字节中除了0x00和0xFF之外的任何值。他不必要地创造性地解决了这个问题。首先,他通过编写表达式(d >> 24)验证了最高有效位为非零。有此表达式的声明,但更有趣的是解析表达式的右侧:((d >> 24)+1)。程序员将高8位移位到低字节。同时,它计算出最高有效符号位在所有其他位中重复。即
如果变量d为0b11111111'00000000'00000000'00000000,则在移位后,我们将得到值0b11111111'11111111'11111111'11111111。将int类型的值0xFFFFFFFF加1 ,程序员计划获得0。即:-1 + 1 = 0。因此,他使用表达式((d >> 24)+1)检查并非所有的高8位都等于1。我知道这是相当困难的,因此请不要着急并尝试找出工作原理和方法:)。现在,让我们看看这段代码有什么问题。移位时,最高有效符号位不一定“被抹上”。这是标准所说的:“ E1 >> E2的值是E1右移E2位的位置。如果E1具有无符号类型,或者E1具有带符号类型且非负值,则结果的值是E1 / 2 / E2的商的整数部分。如果E1具有带符号的类型和负值,则结果值是实现定义的。”最新报价对我们很重要。因此,我们遇到了实现定义的行为。此代码将如何工作取决于微处理器体系结构和编译器的实现。移位后,最高位可能很好地出现零,然后表达式((d >> 24)+1)始终不同于0,即永远是真实的价值。因此得出结论:无需明智。例如,如果您编写如下代码,则代码将变得更加可靠和易于理解: GGLfixed gglFastDivx(GGLfixed n, GGLfixed d) { uint32_t hibits = static_cast<uint32_t>(d) >> 24; if (hibits != 0x00 && hibits != 0xFF) { n >>= 8; d >>= 8; } return gglMulx(n, gglRecip(d)); }
可能我建议的方法不是理想的选择,但是在此代码中,实现没有定义任何行为,并且使读者更容易理解所检查的内容。您应得一杯咖啡或茶。分心并继续:我们正在等待一个有趣的未指定行为案例。在面试时,作为向申请人提出的第一个问题,我问以下问题:“ printf函数将打印什么,为什么打印?” int i = 5; printf("%d,%d", i++, i++)
正确答案:这是未指定的行为。没有定义调用函数时计算实际参数的过程。有时,我什至演示了在Visual C ++的帮助下编译的这段代码在屏幕上显示“ 6.5”,这使知识和精神薄弱的新手完全迷惑了:)。这似乎是一个牵强的问题。但是不可以,这种代码可以在严肃的应用程序中找到,例如,在Android代码中。 bool ComposerClient::CommandReader::parseSetLayerCursorPosition( uint16_t length) { if (length != CommandWriterBase::kSetLayerCursorPositionLength) { return false; } auto err = mHal.setLayerCursorPosition(mDisplay, mLayer, readSigned(), readSigned()); if (err != Error::NONE) { mWriter.setError(getCommandLoc(), err); } return true; }
PVS-Studio警告:V681 CWE-758语言标准未定义在评估参数时将调用“ readSigned”功能的顺序。ComposerClient.cpp 836我们对以下代码行感兴趣: mHal.setLayerCursorPosition(...., readSigned(), readSigned());
通过调用readSigned函数,将读取两个值。但是无法预测将以什么顺序读取值。这是未指定行为的典型案例。使用静态代码分析器的好处
整篇文章通常都会普及静态代码分析,尤其是我们的PVS-Studio工具。但是,有些错误恰好证明了静态分析的可能性。它们很难通过代码审查来识别,并且只有不疲倦的程序才能轻易注意到它们。考虑几个这样的情况。 const std::map<std::string, int32_t> kBootReasonMap = { .... {"watchdog_sdi_apps_reset", 106}, {"smpl", 107}, {"oem_modem_failed_to_powerup", 108}, {"reboot_normal", 109}, {"oem_lpass_cfg", 110},
PVS-Studio警告:- V766 CWE-462已添加具有相同键“ oem_lpass_cfg”的项目。bootstat.cpp 264
- V766 CWE-462已经添加了具有相同键“ oem_xpu_ns_error”的项目。bootstat.cpp 265
具有相同键的不同值将插入已排序的关联容器(std :: map)。在常见弱点枚举方面,这是CWE-462:关联列表中的重复密钥。缩短了程序文本的位置,并用注释标记了错误,因此错误似乎很明显,但是当您用眼睛阅读此代码时,很难找到此类错误。考虑另一段很难阅读的代码,因为它是相同的类型并且没有兴趣。 MtpResponseCode MyMtpDatabase::getDevicePropertyValue(....) { .... switch (type) { case MTP_TYPE_INT8: packet.putInt8(longValue); break; case MTP_TYPE_UINT8: packet.putUInt8(longValue); break; case MTP_TYPE_INT16: packet.putInt16(longValue); break; case MTP_TYPE_UINT16: packet.putUInt16(longValue); break; case MTP_TYPE_INT32: packet.putInt32(longValue); break; case MTP_TYPE_UINT32: packet.putUInt32(longValue); break; case MTP_TYPE_INT64: packet.putInt64(longValue); break; case MTP_TYPE_UINT64: packet.putUInt64(longValue); break; case MTP_TYPE_INT128: packet.putInt128(longValue); break; case MTP_TYPE_UINT128: packet.putInt128(longValue);
PVS-Studio警告:V525 CWE-682该代码包含相似块的集合。在第620、623、626、629行中检查项目'putInt8','putUInt8','putInt16','putUInt16','putInt32','putUInt32','putInt64','putUInt64','putInt128','putInt128' ,632,635,638,641,644,647 620 android_mtp_MtpDatabase.cpp如果MTP_TYPE_UINT128必须由功能引起putUInt128,不putInt128。本节的最后一个是复制粘贴失败的一个很好的例子。 static void btif_rc_upstreams_evt(....) { .... case AVRC_PDU_REQUEST_CONTINUATION_RSP: { BTIF_TRACE_EVENT( "%s() REQUEST CONTINUATION: target_pdu: 0x%02d", __func__, pavrc_cmd->continu.target_pdu); tAVRC_RESPONSE avrc_rsp; if (p_dev->rc_connected == TRUE) { memset(&(avrc_rsp.continu), 0, sizeof(tAVRC_NEXT_RSP)); avrc_rsp.continu.opcode = opcode_from_pdu(AVRC_PDU_REQUEST_CONTINUATION_RSP); avrc_rsp.continu.pdu = AVRC_PDU_REQUEST_CONTINUATION_RSP; avrc_rsp.continu.status = AVRC_STS_NO_ERROR; avrc_rsp.continu.target_pdu = pavrc_cmd->continu.target_pdu; send_metamsg_rsp(p_dev, -1, label, ctype, &avrc_rsp); } } break; case AVRC_PDU_ABORT_CONTINUATION_RSP: { BTIF_TRACE_EVENT( "%s() ABORT CONTINUATION: target_pdu: 0x%02d", __func__, pavrc_cmd->abort.target_pdu); tAVRC_RESPONSE avrc_rsp; if (p_dev->rc_connected == TRUE) { memset(&(avrc_rsp.abort), 0, sizeof(tAVRC_NEXT_RSP)); avrc_rsp.abort.opcode = opcode_from_pdu(AVRC_PDU_ABORT_CONTINUATION_RSP); avrc_rsp.abort.pdu = AVRC_PDU_ABORT_CONTINUATION_RSP; avrc_rsp.abort.status = AVRC_STS_NO_ERROR; avrc_rsp.abort.target_pdu = pavrc_cmd->continu.target_pdu; send_metamsg_rsp(p_dev, -1, label, ctype, &avrc_rsp); } } break; .... }
建议您在阅读分析仪警告和更多内容之前,先查找错误。为了确保您不会意外地立即阅读答案,请使用以下图片来转移您的注意力。如果您对带有铭文Java的鸡蛋的含义感兴趣,那么您来了。因此,我希望您喜欢打字错误。现在是时候向分析器发出警告了:V778 CWE-682发现了两个类似的代码片段。也许这是一个错字,应该使用“ abort”变量而不是“ continu”。btif_rc.cc 1554显然,该代码是使用Copy-Paste方法编写的,并且像往常一样,一个人在编辑复制的代码片段的过程中不能专心。结果,最终,他没有将“ continue ” 替换为“ abort ”。即
在第二块中应写成: avrc_rsp.abort.target_pdu = pavrc_cmd->abort.target_pdu;
这种情况完全属于“ 最后一行效果 ” 的定义,因为在结尾处错误地替换了名称。Facepalm
一个非常有趣的错误与Little-endian和Big-endian数据格式之间的转换有关(请参阅Byte order)。 inline uint32_t bswap32(uint32_t pData) { return (((pData & 0xFF000000) >> 24) | ((pData & 0x00FF0000) >> 8) | ((pData & 0x0000FF00) << 8) | ((pData & 0x000000FF) << 24)); } bool ELFAttribute::merge(....) { .... uint32_t subsection_length = *reinterpret_cast<const uint32_t*>(subsection_data); if (llvm::sys::IsLittleEndianHost != m_Config.targets().isLittleEndian()) bswap32(subsection_length); .... }
PVS-Studio警告:V530 CWE-252需要使用功能“ bswap32”的返回值。ELFAttribute.cpp 84 没有关于bswap32函数的投诉。但是使用不正确: bswap32(subsection_length);
作者建议将变量通过引用传递给函数,然后在此处进行更改。但是,必须使用该函数返回的值。结果,不发生数据转换。分析仪将此错误识别为CWE-252:未检查的返回值。但是,实际上,在这里调用CWE-198更合适:使用不正确的字节顺序。不幸的是,分析器无法从高层次的角度理解错误是什么。但是,这不会阻止他识别代码中的这一严重缺陷。正确的代码是: subsection_length = bswap32(subsection_length);
Android代码中还有另外三个错误相同的地方:- V530 CWE-252需要使用功能“ bswap32”的返回值。ELFAttribute.cpp 218
- V530 CWE-252需要使用功能“ bswap32”的返回值。ELFAttribute.cpp 346
- V530 CWE-252需要使用功能“ bswap32”的返回值。ELFAttribute.cpp 352
为避免此类错误,建议使用[[nodiscard]]属性。此属性用于指示调用函数时必须使用函数的返回值。因此,如果您这样写: [[nodiscard]] inline uint32_t bswap32(uint32_t pData) { ... }
那么即使在编译文件的阶段,也会检测到错误。您可以从我同事的文章“ C ++ 17 ”中了解更多关于一些新的有用属性的信息。无法访问的代码
在编程和编译器理论中,无法到达的代码是程序代码的一部分,在任何情况下都无法执行,因为它在控制流程图中无法到达。从常见弱点枚举的角度来看,它是CWE-561:死代码。 virtual sp<IEffect> createEffect(....) { .... if (pDesc == NULL) { return effect; if (status != NULL) { *status = BAD_VALUE; } } .... }
PVS-Studio警告:V779 CWE-561检测不到代码。可能存在错误。IAudioFlinger.cpp 733运营商的回报显然必须位于下方。此类型的其他错误:- V779 CWE-561检测不到代码。可能存在错误。bta_hf_client_main.cc 612
- V779 CWE-561检测不到代码。可能存在错误。android_media_ImageReader.cpp 468
- V779 CWE-561检测不到代码。可能存在错误。AMPEG4AudioAssembler.cpp 187
打破
忘记在开关内部中断是C和C ++程序员的经典错误。为了解决这个问题,C ++ 17中出现了一个有用的注释[[fallthrough]]。您可以在我的文章“ Break and fallthrough ”中阅读有关此错误和[[fallthrough]]的更多信息。但是,尽管世界充满了不使用[[fallthrough]]的旧代码,但是PVS-Studio对您有用。考虑一下Android中发现的一些错误。根据常见弱点枚举,这些错误被分类为CWE-484:交换机中的省略中断语句。 bool A2dpCodecConfigLdac::setCodecConfig(....) { .... case BTAV_A2DP_CODEC_SAMPLE_RATE_192000: if (sampleRate & A2DP_LDAC_SAMPLING_FREQ_192000) { result_config_cie.sampleRate = A2DP_LDAC_SAMPLING_FREQ_192000; codec_capability_.sample_rate = codec_user_config_.sample_rate; codec_config_.sample_rate = codec_user_config_.sample_rate; } case BTAV_A2DP_CODEC_SAMPLE_RATE_16000: case BTAV_A2DP_CODEC_SAMPLE_RATE_24000: case BTAV_A2DP_CODEC_SAMPLE_RATE_NONE: codec_capability_.sample_rate = BTAV_A2DP_CODEC_SAMPLE_RATE_NONE; codec_config_.sample_rate = BTAV_A2DP_CODEC_SAMPLE_RATE_NONE; break; .... }
PVS-Studio警告:V796 CWE-484 switch语句中可能缺少'break'语句。a2dp_vendor_ldac.cc 912我认为该错误无需解释。我仅注意到,经常会以多种方式检测到代码中的异常。例如,V519警告也检测到此错误:- V519 CWE-563'codec_capability_.sample_rate'变量已连续两次分配值。也许这是一个错误。检查行:910、916。a2dp_vendor_ldac.cc 916
- V519 CWE-563'codec_config_.sample_rate'变量已连续两次分配值。也许这是一个错误。检查线:911、917。a2dp_vendor_ldac.cc 917
还有更多错误: Return<void> EffectsFactory::getAllDescriptors(....) { .... switch (status) { case -ENOSYS: {
PVS-Studio警告:V796 CWE-484 switch语句中可能缺少'break'语句。EffectsFactory.cpp 118 int Reverb_getParameter(....) { .... case REVERB_PARAM_REFLECTIONS_LEVEL: *(uint16_t *)pValue = 0; case REVERB_PARAM_REFLECTIONS_DELAY: *(uint32_t *)pValue = 0; case REVERB_PARAM_REVERB_DELAY: *(uint32_t *)pValue = 0; break; .... }
PVS-Studio警告:V796 CWE-484 switch语句中可能缺少'break'语句。EffectReverb.cpp 1847 static SLresult IAndroidConfiguration_GetConfiguration(....) { .... switch (IObjectToObjectID((thiz)->mThis)) { case SL_OBJECTID_AUDIORECORDER: result = android_audioRecorder_getConfig( (CAudioRecorder *) thiz->mThis, configKey, pValueSize, pConfigValue); break; case SL_OBJECTID_AUDIOPLAYER: result = android_audioPlayer_getConfig( (CAudioPlayer *) thiz->mThis, configKey, pValueSize, pConfigValue); default: result = SL_RESULT_FEATURE_UNSUPPORTED; break; } .... }
PVS-Studio警告:V796 CWE-484 switch语句中可能缺少'break'语句。IAndroidConfiguration.cpp 90错误的内存管理
在这里,我编译了与不正确的内存管理有关的错误。根据常见弱点枚举,此类警告分类为:让我们从返回对已销毁变量的引用的函数开始。 TransformIterator& operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator& operator--(int) { TransformIterator tmp(*this); --*this; return tmp; }
PVS-Studio警告:- V558 CWE-562函数将对临时本地对象tmp的引用返回。transform_iterator.h 77
- V558 CWE-562函数将对临时本地对象tmp的引用返回。transform_iterator.h 92
当函数完成执行时,tmp变量将被销毁,因为它是在堆栈上创建的。因此,函数返回对已销毁(不存在)对象的引用。正确的解决方案是返回值: TransformIterator operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator operator--(int) { TransformIterator tmp(*this); --*this; return tmp; }
考虑更多值得密切关注的可悲代码。 int register_socket_transport( int s, const char* serial, int port, int local) { atransport* t = new atransport(); if (!serial) { char buf[32]; snprintf(buf, sizeof(buf), "T-%p", t); serial = buf; } .... }
PVS-Studio警告:V507 CWE-562指向本地数组'buf'的指针存储在该数组的范围之外。这样的指针将变为无效。 transport.cpp 1030这是危险的代码。如果串行参数的实际值为NULL,则应使用堆栈上的临时缓冲区。当if语句的主体结束时,buf数组将不复存在。创建缓冲区的位置可用于存储在堆栈上创建的其他变量。数据将开始出现混乱的混乱,并且这种错误的后果很难预测。以下错误与创建和销毁对象的不兼容方法有关。 void SensorService::SensorEventConnection::reAllocateCacheLocked(....) { sensors_event_t *eventCache_new; const int new_cache_size = computeMaxCacheSizeLocked(); eventCache_new = new sensors_event_t[new_cache_size]; .... delete mEventCache; mEventCache = eventCache_new; mCacheSize += count; mMaxCacheSize = new_cache_size; }
PVS-Studio警告:V611 CWE-762使用“ new T []”运算符分配了内存,但使用“ delete”运算符释放了内存。考虑检查此代码。最好使用“ delete [] mEventCache;”。检查行:391,384。SensorEventConnection.cpp 391这里的一切都很简单。mEventCache类的成员指向的缓冲区是使用new []运算符分配的。并使用delete运算符释放此内存。这是不正确的,并导致未定义的程序行为。类似错误: aaudio_result_t AAudioServiceEndpointCapture::open(....) { .... delete mDistributionBuffer; int distributionBufferSizeBytes = getStreamInternal()->getFramesPerBurst() * getStreamInternal()->getBytesPerFrame(); mDistributionBuffer = new uint8_t[distributionBufferSizeBytes]; .... }
PVS-Studio警告:V611 CWE-762使用“ new T []”运算符分配了内存,但使用“ delete”运算符释放了内存。考虑检查此代码。最好使用“ delete [] mDistributionBuffer;”。AAudioServiceEndpointCapture.cpp 50我认为该错误无需解释。以下情况更有趣,但错误的本质完全相同。 struct HeifFrameInfo { .... void set(....) { .... mIccData.reset(new uint8_t[iccSize]); .... } .... std::unique_ptr<uint8_t> mIccData; };
V554 CWE-762错误使用unique_ptr。 分配给'new []'的内存将使用'delete'清除。
HeifDecoderAPI.h 62默认情况下,智能指针类std :: unique_ptr调用delete运算符销毁一个对象。但是,在set函数中,使用new []运算符分配内存。正确的选项:
std::unique_ptr<uint8_t[]> mIccData;
其他错误:
- V554 CWE-762错误使用unique_ptr。 分配给'new []'的内存将使用'delete'清除。 第949章
- V554 CWE-762错误使用unique_ptr。 分配给'new []'的内存将使用'delete'清除。 atrace.cpp 950
- V554 CWE-762错误使用unique_ptr。 分配给'new []'的内存将使用'delete'清除。 HeifDecoderImpl.cpp 102
- V554 CWE-762错误使用unique_ptr。 分配给'new []'的内存将使用'delete'清除。 Heif解码器Impl.cpp 166
- V554 CWE-762错误使用unique_ptr。 分配给'new []'的内存将使用'delete'清除。 ColorSpace.cpp 360
内存泄漏错误将完成本节。令人不愉快的是,有20多个这样的错误,在我看来,这些错误可能是非常痛苦的缺陷,导致在长时间连续运行操作系统期间,可用内存逐渐减少。 Asset* Asset::createFromUncompressedMap(FileMap* dataMap, AccessMode mode) { _FileAsset* pAsset; status_t result; pAsset = new _FileAsset; result = pAsset->openChunk(dataMap); if (result != NO_ERROR) return NULL; pAsset->mAccessMode = mode; return pAsset; }
PVS-Studio警告:V773 CWE-401在不释放“ pAsset”指针的情况下退出了该功能。 可能发生内存泄漏。
Asset.cpp 296如果无法打开某个块,则函数将退出而不破坏对象,指向该对象的指针存储在pAsset变量中。结果,将发生内存泄漏。其他错误类似,因此我认为没有理由在文章中考虑它们。有兴趣的人可以在文件Android_V773.txt中查看其他警告。出国数组
有很多错误的模式会导致数组溢出。对于Android,我仅检测到以下类型的一种错误模式: if (i < 0 || i > MAX) return; A[i] = x;
在C和C ++中,数组单元格从0开始编号,因此数组中元素的最大索引必须比数组本身的大小小1。正确的检查应如下所示: if (i < 0 || i >= MAX) return; A[i] = x;
根据常见弱点枚举,溢出数组的类别为CWE-119:内存缓冲区范围内的操作限制不当。看看这些错误在Android代码中的样子。 static btif_hf_cb_t btif_hf_cb[BTA_AG_MAX_NUM_CLIENTS]; static bool IsSlcConnected(RawAddress* bd_addr) { if (!bd_addr) { LOG(WARNING) << __func__ << ": bd_addr is null"; return false; } int idx = btif_hf_idx_by_bdaddr(bd_addr); if (idx < 0 || idx > BTA_AG_MAX_NUM_CLIENTS) { LOG(WARNING) << __func__ << ": invalid index " << idx << " for " << *bd_addr; return false; } return btif_hf_cb[idx].state == BTHF_CONNECTION_STATE_SLC_CONNECTED; }
PVS-Studio警告:V557 CWE-119阵列可能超限。“ idx”索引的值可能达到6。btif_hf.cc 277正确的检查选项: if (idx < 0 || idx >= BTA_AG_MAX_NUM_CLIENTS) {
完全相同的错误又发现了两件事:- V557 CWE-119阵列可能超限。“ idx”索引的值可能达到6。btif_hf.cc 869
- V557 CWE-119阵列可能超限。“索引”索引的值可能达到6。btif_rc.cc 374
循环中断
有许多方法可以编写故障循环。在Android代码中,根据常见弱点枚举,我遇到了一些错误,这些错误可以归类为:当然,与此同时,在编写循环时,还有其他方法可以“拍自己的脚”,但是这次却没有遇到我。 int main(int argc, char **argv) { .... char c; printf("%s is already in *.base_fs format, just ..... ", ....); rewind(blk_alloc_file); while ((c = fgetc(blk_alloc_file)) != EOF) { fputc(c, base_fs_file); } .... }
PVS-Studio警告:V739 CWE-20 EOF不应与“ char”类型的值进行比较。 '(c = fgetc(blk_alloc_file))'应该是'int'类型的。 blk_alloc_to_base_fs.c 61分析器检测到EOF常量与char类型的变量进行比较。让我们看看为什么此代码不正确。fgetc函数返回一个int值,即:它可以返回一个从0到255或EOF(-1)的数字。读取的值放在char类型的变量中。因此,值为0xFF(255)的字符变为-1,并以与文件末尾(EOF)相同的方式进行解释。由于这些错误,使用扩展ASCII码的用户有时会遇到程序错误地处理其字母表中的一个字符的情况。例如,编码为Windows-1251的俄语字母的最后一个字母仅具有代码0xFF,并且在某些程序中被视为文件的末尾。综上所述,可以说停止循环的条件写得不正确。为了解决这种情况,变量c必须为int类型。我们继续考虑使用for语句时的更熟悉的错误。 status_t AudioPolicyManager::registerPolicyMixes(....) { .... for (size_t i = 0; i < mixes.size(); i++) { .... for (size_t j = 0; i < mHwModules.size(); j++) {
PVS-Studio警告:V534 CWE-691可能在“ for”运算符内比较了错误的变量。考虑查看“ i”。 AudioPolicyManager.cpp 2489由于嵌套循环中有错字,该条件使用变量i,尽管您必须使用变量j。结果,变量j不受控制地增加,随着时间的流逝,将导致mHwModules数组超出范围。接下来将要发生的事情无法预测,因为会有不确定的程序行为。顺便说一句,这个带有错误的片段已完全复制到另一个函数。因此,分析器在此处发现了完全相同的错误:AudioPolicyManager.cpp 2586。还有3个对我来说非常可疑的代码段。但是,我不能假定此代码肯定是错误的,因为那里存在复杂的逻辑。无论如何,我必须注意此代码,以便作者对其进行检查。第一个片段: void ce_t3t_handle_check_cmd(....) { .... for (i = 0; i < p_cb->cur_cmd.num_blocks; i++) { .... for (i = 0; i < T3T_MSG_NDEF_ATTR_INFO_SIZE; i++) { checksum += p_temp[i]; } .... } .... }
PVS-Studio警告:V535 CWE-691变量“ i”正用于此循环和外部循环。检查行:398,452。ce_t3t.cc 452请注意,变量i用于外部和内部循环。另外两个类似的分析器触发器:- V535 CWE-691变量“ xx”用于该循环和外部循环。检查行:801、807。sdp_db.cc 807
- V535 CWE-691变量“ xx”用于该循环和外部循环。检查行:424、438。nfa_hci_act.cc 438
你还不累吗 我建议暂停并下载PVS-Studio以尝试检查您的项目。现在让我们继续。 #define NFA_HCI_LAST_PROP_GATE 0xFF tNFA_HCI_DYN_GATE* nfa_hciu_alloc_gate(uint8_t gate_id, tNFA_HANDLE app_handle) { .... for (gate_id = NFA_HCI_FIRST_HOST_SPECIFIC_GENERIC_GATE; gate_id <= NFA_HCI_LAST_PROP_GATE; gate_id++) { if (gate_id == NFA_HCI_CONNECTIVITY_GATE) gate_id++; if (nfa_hciu_find_gate_by_gid(gate_id) == NULL) break; } if (gate_id > NFA_HCI_LAST_PROP_GATE) { LOG(ERROR) << StringPrintf( "nfa_hci_alloc_gate - no free Gate ID: %u " "App Handle: 0x%04x", gate_id, app_handle); return (NULL); } .... }
PVS-Studio警告:V654 CWE-834循环的条件“ gate_id <= 0xFF”始终为真。nfa_hci_utils.cc 248请注意以下几点:- 常量NFA_HCI_LAST_PROP_GATE等于0xFF。
- uint8_t类型的变量用作循环计数器。因此,此变量的值范围是[0..0xFF]。
事实证明,条件gate_id <= NFA_HCI_LAST_PROP_GATE始终为true,并且无法停止循环的执行。分析器将此错误分类为CWE-834,但也可以将其解释为CWE-571:表达式始终为True。循环中的下一个错误与未定义的行为有关。 status_t SimpleDecodingSource::doRead(....) { .... for (int retries = 0; ++retries; ) { .... }
PVS-Studio警告:V654 CWE-834循环的条件“ ++重试”始终为true。SimpleDecodingSource.cpp 226显然,程序员希望retry变量采用int变量的所有可能值,然后循环才结束。当表达式++ retries为0 时,循环应停止。这仅在变量溢出时才有可能。由于变量是带符号的类型,因此其溢出会导致未定义的行为。因此,此代码不正确,并可能导致不可预测的后果。例如,编译器拥有删除支票的全部权利,仅保留递增计数器的指令。这是本节中的最后一个错误。 status_t Check(const std::string& source) { .... int pass = 1; .... do { .... switch(rc) { case 0: SLOGI("Filesystem check completed OK"); return 0; case 2: SLOGE("Filesystem check failed (not a FAT filesystem)"); errno = ENODATA; return -1; case 4: if (pass++ <= 3) { SLOGW("Filesystem modified - rechecking (pass %d)", pass); continue;
PVS-Studio警告:V696 CWE-670因为条件始终为假,所以“继续”操作符将终止“({FALSE)”时执行“ do {...}”循环。检查行:105,121。Vfat.cpp 105在我们面前的是以下形式的循环: do { .... if (x) continue; .... } while (0)
为了执行重复的迭代,程序员使用continue语句。错了
Continue语句不会立即恢复循环,而是继续检查条件。由于条件始终为假,因此在任何情况下循环仅执行一次。要解决该错误,可以重写代码,例如: for (;;) { .... if (x) continue; .... break; }
可变重新分配
一个非常常见的错误是在使用前一个值之前重写变量。大多数情况下,此类错误是由于输入错误或复制粘贴不成功而发生的。根据常见弱点枚举,此类错误分类为CWE-563:不使用变量的分配。在Android中并非没有这样的错误。 status_t XMLNode::flatten_node(....) const { .... memset(&namespaceExt, 0, sizeof(namespaceExt)); if (mNamespacePrefix.size() > 0) { namespaceExt.prefix.index = htodl(strings.offsetForString(mNamespacePrefix)); } else { namespaceExt.prefix.index = htodl((uint32_t)-1); } namespaceExt.prefix.index = htodl(strings.offsetForString(mNamespacePrefix)); namespaceExt.uri.index = htodl(strings.offsetForString(mNamespaceUri)); .... }
PVS-Studio警告:V519 CWE-563'namespaceExt.prefix.index'变量已连续两次分配值。也许这是一个错误。检查行:1535、1539。XMLNode.cpp 1539为了突出显示错误的实质,我将编写一个伪代码: if (a > 0) X = 1; else X = 2; X = 1;
无论条件如何,变量X(在当前代码中为namespaceExt.prefix.index)始终将分配一个值。 bool AudioFlinger::RecordThread::threadLoop() { .... size_t framesToRead = mBufferSize / mFrameSize; framesToRead = min(mRsmpInFramesOA - rear, mRsmpInFramesP2 / 2); .... }
PVS-Studio警告:V519 CWE-563'framesToRead'变量已连续两次分配值。也许这是一个错误。检查行:6341,6342。Threads.cpp 6342不清楚为什么在声明时必须初始化变量,如果立即将另一个值写入变量。这里出了点问题。 void SchedulingLatencyVisitorARM::VisitArrayGet(....) { .... if (index->IsConstant()) { last_visited_latency_ = kArmMemoryLoadLatency; } else { if (has_intermediate_address) { } else { last_visited_internal_latency_ += kArmIntegerOpLatency; } last_visited_internal_latency_ = kArmMemoryLoadLatency; } .... }
PVS-Studio警告:V519 CWE-563'last_visited_internal_latency_'变量已连续两次分配值。也许这是一个错误。检查行:680,682。scheduler_arm.cc 682非常奇怪,毫无意义的代码。我冒险建议应该将其写成: last_visited_internal_latency_ += kArmMemoryLoadLatency;
最后一个错误,展示了分析器如何孜孜不倦地发现即使仔细检查代码也最有可能被跳过的错误。 void multiprecision_fast_mod(uint32_t* c, uint32_t* a) { uint32_t U; uint32_t V; .... c[0] += U; V = c[0] < U; c[1] += V; V = c[1] < V; c[2] += V;
PVS-Studio警告:V519 CWE-563'V'变量已连续两次分配值。也许这是一个错误。检查行:307,309。p_256_multprecision.cc 309代码太“ 让人眼花”乱”,以至于我不想理解它。这很明显:代码中有一个错字,我在注释中突出了该错字。其他错误
存在零散的错误,因此没有必要单独撰写章节。但是,它们与前面考虑的一样有趣和阴险。优先行动 void TagMonitor::parseTagsToMonitor(String8 tagNames) { std::lock_guard<std::mutex> lock(mMonitorMutex);
PVS-Studio警告:V593 CWE-783考虑考虑'A = B!= C'类型的表达。该表达式的计算公式如下:“ A =(B!= C)”。 TagMonitor.cpp 50常见弱点枚举:CWE-783:操作员优先逻辑错误。程序员构思了以下内容。搜索子字符串“ 3a”,并将该子字符串的位置写入idx变量。如果找到子字符串(idx!= -1),则代码开始运行,该代码使用idx变量的值。不幸的是,程序员对操作的优先级感到困惑。实际上,检查工作如下: if (ssize_t idx = (tagNames.find("3a") != -1))
首先,它检查字符串中是否存在子字符串“ 3a”,并将结果false / true放在idx变量中。结果,idx变量的值为0或1。如果条件为true(idx变量为1),则使用idx变量的逻辑开始执行。始终等于1的变量将导致错误的程序行为。如果您根据条件对变量进行初始化,则可以修复错误: ssize_t idx = tagNames.find("3a"); if (idx != -1)
新版本的C ++ 17还允许您编写: if (ssize_t idx = tagNames.find("3a"); idx != -1)
无效的构造函数 struct HearingDevice { .... HearingDevice() { HearingDevice(RawAddress::kEmpty, false); } .... };
PVS-Studio警告:V603 CWE-665已创建对象,但未使用该对象。如果要调用构造函数,则应使用“ this-> HearingDevice :: HearingDevice(....)”。听力_aid.cc 176常见弱点枚举:CWE-665:不正确的初始化。试图显式调用构造函数初始化对象时,程序员常常会犯错误。该类中有两个构造函数。为了减小源代码的大小,程序员决定从另一个调用一个构造函数。但是这段代码根本不符合开发人员的期望。发生以下情况。创建一个新的HearingDevice类型的未命名对象,然后销毁它。结果,类字段保持未初始化。要解决该错误,可以使用委托的构造函数(此功能出现在C ++ 11中)。正确的代码是: HearingDevice() : HearingDevice(RawAddress::kEmpty, false) { }
函数不返回值 int NET_RecvFrom(int s, void *buf, int len, unsigned int flags, struct sockaddr *from, int *fromlen) { socklen_t socklen = *fromlen; BLOCKING_IO_RETURN_INT( s, recvfrom(s, buf, len, flags, from, &socklen) ); *fromlen = socklen; }
PVS-Studio警告:V591 CWE-393非无效函数应返回一个值。linux_close.cpp 139常见弱点枚举:CWE-393:返回错误的状态代码。该函数将返回一个随机值。另一个此类错误:V591 CWE-393非无效函数应返回一个值。linux_close.cpp 158结构尺寸计算不正确 int MtpFfsHandle::handleControlRequest(....) { .... struct mtp_device_status *st = reinterpret_cast<struct mtp_device_status*>(buf.data()); st->wLength = htole16(sizeof(st)); .... }
PVS-Studio警告:V568'sizeof()'运算符会评估指向类的指针的大小,但不会评估'st'类对象的大小,这很奇怪。MtpFfsHandle.cpp 251我确信他们想将结构的大小而不是指针的大小放到wLength成员变量中。然后正确的代码应如下所示: st->wLength = htole16(sizeof(*st));
类似的分析仪响应:- V568很奇怪,'sizeof()'运算符会评估指向类的指针的大小,而不是'cacheinfo'类对象的大小。NetlinkEvent.cpp 220
- V568很奇怪,'sizeof()'运算符会评估指向类的指针的大小,而不是'page-> next'类对象的大小。linker_block_allocator.cpp 146
- V568奇怪的是sizeof()运算符的参数是'&session_id'表达式。参考文献ril.c 1775
无意义的位操作 #define EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR 0x00000001 #define EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR 0x00000002 #define EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR 0x00000004 EGLContext eglCreateContext(....) { .... case EGL_CONTEXT_FLAGS_KHR: if ((attrib_val | EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR) || (attrib_val | EGL_CONTEXT_OPENGL_FORWARD_C....) || (attrib_val | EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR)) { context_flags = attrib_val; } else { RETURN_ERROR(EGL_NO_CONTEXT,EGL_BAD_ATTRIBUTE); } .... }
PVS-Studio警告:V617 CWE-480考虑检查状况。“ |”的“ 0x00000001”参数 按位运算包含一个非零值。egl.cpp 1329常见弱点枚举:CWE-480:使用错误的运算符。形式(A | 1)||的表达式 (A | 2)|| (A | 4)没有意义,因为结果将始终为真。实际上,您需要使用&运算符,然后代码才有意义: if ((attrib_val & EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR) || (attrib_val & EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR) || (attrib_val & EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR))
类似错误:V617 CWE-480考虑检查状况。 “ |”的“ 0x00000001”参数按位运算包含一个非零值。 egl.cpp 1338错误的移位 template <typename AddressType> struct RegsInfo { .... uint64_t saved_reg_map = 0; AddressType saved_regs[64]; .... inline AddressType* Save(uint32_t reg) { if (reg > sizeof(saved_regs) / sizeof(AddressType)) { abort(); } saved_reg_map |= 1 << reg; saved_regs[reg] = (*regs)[reg]; return &(*regs)[reg]; } .... }
PVS-Studio警告:V629 CWE-190考虑检查'1 << reg'表达式。32位值的位移,随后扩展为64位类型。RegsInfo.h 47常见弱点枚举:CWE-190:整数溢出或环绕。移位1 << reg,变量reg 的值在[0..63]范围内。该表达式用于获得不同的2度,从2 ^ 0到2 ^ 63结束。该代码不起作用。事实是数字文字1具有32位int类型。因此,获得大于1 ^ 31的值将不起作用。移至更大的值会导致变量溢出和出现不确定的行为。正确的代码是: saved_reg_map |= static_cast<uint64_t>(1) << reg;
或: saved_reg_map |= 1ULL << reg;
字符串被复制到自己。 void PCLmGenerator::writeJobTicket() {
PVS-Studio警告:- V549 CWE-688'strcpy'函数的第一个参数等于第二个参数。genPCLm.cpp 1181
- V549 CWE-688'strcpy'函数的第一个参数等于第二个参数。第1182章
根据常见弱点枚举的分类:CWE-688:带有错误变量或引用作为参数的函数调用。出于某种原因,字符串会复制到自身。最有可能的是这里有些错别字。使用未初始化的变量 void mca_set_cfg_by_tbl(....) { tMCA_DCB* p_dcb; const tL2CAP_FCR_OPTS* p_opt; tMCA_FCS_OPT fcs = MCA_FCS_NONE; if (p_tbl->tcid == MCA_CTRL_TCID) { p_opt = &mca_l2c_fcr_opts_def; } else { p_dcb = mca_dcb_by_hdl(p_tbl->cb_idx); if (p_dcb) { p_opt = &p_dcb->p_chnl_cfg->fcr_opt; fcs = p_dcb->p_chnl_cfg->fcs; } } memset(p_cfg, 0, sizeof(tL2CAP_CFG_INFO)); p_cfg->mtu_present = true; p_cfg->mtu = p_tbl->my_mtu; p_cfg->fcr_present = true; memcpy(&p_cfg->fcr, p_opt, sizeof(tL2CAP_FCR_OPTS));
PVS-Studio警告:V614 CWE-824使用了潜在的未初始化指针'p_opt'。考虑检查“ memcpy”函数的第二个实际参数。 mca_main.cc 252常见弱点枚举:CWE-824:访问未初始化的指针。如果p_tbl-> tcid!= MCA_CTRL_TCID和p_dcb == nullptr,则指针p_opt将保持未初始化状态。陌生使用未初始化的变量 struct timespec { __time_t tv_sec; long int tv_nsec; }; static inline timespec NsToTimespec(int64_t ns) { timespec t; int32_t remainder; t.tv_sec = ns / kNanosPerSecond; remainder = ns % kNanosPerSecond; if (remainder < 0) { t.tv_nsec--; remainder += kNanosPerSecond; } t.tv_nsec = remainder; return t; }
PVS-Studio警告:V614 CWE-457使用了未初始化的变量't.tv_nsec'。clock_ns.h 55常见弱点枚举:CWE-457:使用未初始化的变量。在减小变量t.tv_nsec时,它尚未初始化。该变量稍后进行初始化:t.tv_nsec =余数; 。
这里有些东西显然很混乱。过度表达 void bta_dm_co_ble_io_req(....) { .... *p_auth_req = bte_appl_cfg.ble_auth_req | (bte_appl_cfg.ble_auth_req & 0x04) | ((*p_auth_req) & 0x04); .... }
PVS-Studio警告:V578检测到奇怪的按位操作。考虑进行验证。bta_dm_co.cc 259此表达式是多余的。如果删除子表达式(bte_appl_cfg.ble_auth_req&0x04),则表达式的结果将保持不变。也许这里有些错字。处理泄漏 bool RSReflectionCpp::genEncodedBitCode() { FILE *pfin = fopen(mBitCodeFilePath.c_str(), "rb"); if (pfin == nullptr) { fprintf(stderr, "Error: could not read file %s\n", mBitCodeFilePath.c_str()); return false; } unsigned char buf[16]; int read_length; mOut.indent() << "static const unsigned char __txt[] ="; mOut.startBlock(); while ((read_length = fread(buf, 1, sizeof(buf), pfin)) > 0) { mOut.indent(); for (int i = 0; i < read_length; i++) { char buf2[16]; snprintf(buf2, sizeof(buf2), "0x%02x,", buf[i]); mOut << buf2; } mOut << "\n"; } mOut.endBlock(true); mOut << "\n"; return true; }
PVS-Studio警告:V773 CWE-401在不释放“ pfin”手柄的情况下退出了该功能。 资源泄漏是可能的。
slang_rs_reflection_cpp.cpp 448分析仪根据常见弱点枚举将该错误分类为:CWE-401:在删除上一个引用之前内存释放不当(“内存泄漏”)。但是,在这里发布CWE-775:在有效生命周期后缺少文件描述符或句柄的发布会更正确。我将指示我的同事更正PVS-Studio中的此缺陷。描述PFIN从来没有公布过。只是忘了最后调用fclose函数。一个令人不愉快的错误,它可能很快耗尽整个可用描述符的供应,此后将无法打开新文件。结论
如您所见,即使在像Android这样的知名项目中,PVS-Studio分析仪也很容易发现许多错误和潜在的漏洞。总结发现哪些弱点(潜在漏洞):- CWE-14:清除代码的编译器删除缓冲区
- CWE-20:输入验证不正确
- CWE-119:内存缓冲区范围内的操作限制不当
- CWE-190:整数溢出或环绕
- CWE-198:使用不正确的字节顺序
- CWE-393:返回错误的状态码
- CWE-401:删除最后一个引用之前内存释放不当(“内存泄漏”)
- CWE-457:使用未初始化的变量
- CWE-462:关联列表中的重复密钥
- CWE-480:使用不正确的运算符
- CWE-484:交换机中的省略中断语句
- CWE-561:无效代码
- CWE-562:堆栈变量地址的返回
- CWE-563: Assignment to Variable without Use
- CWE-570: Expression is Always False
- CWE-571: Expression is Always True
- CWE-476: NULL Pointer Dereference
- CWE-628: Function Call with Incorrectly Specified Arguments
- CWE-665: Improper Initialization
- CWE-670: Always-Incorrect Control Flow Implementation
- CWE-682: Incorrect Calculation
- CWE-688: Function Call With Incorrect Variable or Reference as Argument
- CWE-690: Unchecked Return Value to NULL Pointer Dereference
- CWE-691: Insufficient Control Flow Management
- CWE-758: Reliance on Undefined, Unspecified, or Implementation-Defined Behavior
- CWE-762: Mismatched Memory Management Routines
- CWE-775: Missing Release of File Descriptor or Handle after Effective Lifetime
- CWE-783: Operator Precedence Logic Error
- CWE-824: Access of Uninitialized Pointer
- CWE-834: Excessive Iteration
在本文中,我总共描述了490个潜在漏洞。实际上,分析器能够识别更多的报告,但是,正如我之前所写,我没有发现更仔细地研究报告的实力。在C和C ++中,经过测试的代码库的大小约为2,168,000行代码。在这些评论中,有14.4%是评论。总共,我们获得了大约1,855,000行的干净代码。因此,我们有490个CWE,可用于1,855,000行代码。事实证明,对于Android项目中的每4000行代码,PVS-Studio分析仪能够检测到1个以上的弱点(潜在漏洞)。我很高兴为代码分析器提供良好的结果。感谢您的关注!
祝大家没有代码,并建议执行以下操作:- 下载 PVS-Studio并检查工作草案。
- : : .
- , : twitter , RSS , vk.com .

如果您想与说英语的读者分享这篇文章,请使用以下链接:Andrey Karpov。
We Checked the Android Source Codes by PVS-Studio or Nothing is Perfect