对微控制器进行编程的任何人都可能了解FreeRTOS，或者至少听说过该操作系统。 亚马逊开发人员决定增强该操作系统与AWS Internet of Things服务一起使用的能力。 这就是Amazon FreeRTOS出现的方式。 我们是PVS-Studio静态代码分析器的开发人员，已通过邮件和评论的形式要求我们检查这些项目。 好了，现在就满足您的要求。 继续阅读以找出其中的内容。
简要介绍项目
首先，我将向您介绍一些正在测试的项目的先驱-FreeRTOS（源代码可在此处通过
link获得 ）。 如Wikipedia所述，FreeRTOS是嵌入式系统的实时多任务操作系统。
它是用旧的C语言编写的，这并不奇怪-该操作系统应在微控制器的典型条件下工作：处理能力低，RAM量少等。 C语言允许您以较低的级别使用资源并具有高性能，因此最适合开发这样的OS。
现在回到亚马逊，亚马逊一直在发展各种有前途的方向。 例如，亚马逊正在开发Amazon Lumberyard AAA引擎，我们
也对此
进行了检查 。
这样的方向之一是物联网（IoT）。 为了在此领域发展，亚马逊决定编写自己的操作系统-他们以FreeRTOS核心为基础。
最终的系统Amazon FreeRTOS被定位为“提供与Amazon Web Services的安全连接，例如AWS IoT Core或AWS IoT Greengrass”。 该项目的源代码
可在Github上获得。
在本文中，我们将了解FreeRTOS中是否存在错误以及就静态代码分析而言，Amazon操作系统的安全性如何。
检查过程
使用自动错误查找工具-PVS-Studio静态代码分析器执行检查。 它能够检测用C，C ++，C＃和Java编写的程序中的错误。
在分析之前，我们必须构建项目。 这样，我将确信我具有所有需要的依赖项，并且可以检查该项目了。 人们可以通过多种方式检查项目，例如，使用编译监视系统。 在这种情况下，我使用Visual Studio插件进行了分析-很好的是，两个项目的存储库都包含可轻松在Windows下构建的项目文件集。
我只需要构建项目以确保已准备好一切即可进行检查。 接下来，我进行分析，瞧！ -我面前有一份现成的分析仪报告。
这些项目中包含的第三方库也可能包含错误，它们当然也会影响程序。 但是，为了叙述的纯正，我从分析中排除了它们。
因此，分析了项目，收到了报告，突出了有趣的错误。 现在该得到他们的评论！
FreeRTOS隐藏了什么
最初，我希望写两篇单独的文章：每个操作系统一篇。 我已经在搓手了吗？ 当我准备写一篇关于FreeRTOS的好文章时。 预计至少会发现两个多汁的bug（例如
CWE-457 ），我正在查看分析仪的稀疏警告，却发现什么都没有。 我没有发现任何有趣的错误。
分析仪发出的许多警告与FreeRTOS不相关。 例如，此类警告是64位缺陷，例如将
size_t转换为
uint32_t 。 这与FreeRTOS旨在在指针大小不大于32位的设备上工作有关。
我已经彻底检查了所有
V1027警告，这些警告指示在指向不相关结构的指针之间进行
强制转换 。 如果铸造结构具有相同的对齐方式，则这种铸造是错误的。 而且我还没有发现任何危险的铸件！
所有其他可疑的地方要么与编码风格相关联，要么配备了注释，解释了为什么这样做和为什么不是错误。
因此，我想吸引FreeRTOS开发人员。 伙计们，您真棒！ 我们几乎看不到像您这样干净，高质量的项目。 很高兴阅读干净，整洁且有据可查的代码。 向你们致敬。
即使那天我找不到任何有趣的错误，但我知道我不会止步于此。 我怀着坚定的信心回到家，因为亚马逊的版本将100％具有有趣之处，而且明天我肯定会为本文找到足够的bug。 您可能已经猜到了，我是对的。
Amazon FreeRTOS隐藏的内容
事实证明，亚马逊的系统版本是...稍微说得有点差。 FreeRTOS的遗产仍然很干净，而新的改进隐藏了许多有趣的问题。
有些片段破坏了程序逻辑，有些错误地处理了指针。 在某些地方，代码可能导致未定义的行为，并且在某些情况下，程序员根本不了解他所犯错误的模式。 我什至发现了几个严重的潜在漏洞。
似乎我已经加强了介绍。 让我们开始找出错误！
程序逻辑的破坏
让我们从有问题的地方开始，这些地方显然表明程序的工作方式与程序员预期的不同。 可疑数组处理将首先出现：
static _requestPool_t _requestPool = { 0 }; .... static int _scheduleAsyncRequest(int reqIndex, uint32_t currentRange) { .... _requestPool.pRequestDatas[reqIndex].pConnHandle = &_connHandle; _requestPool.pRequestDatas[reqIndex].pConnConfig = &_connConfig; _requestPool.pRequestDatas[reqIndex].reqNum = reqIndex; _requestPool.pRequestDatas[reqIndex].currRange = currentRange; _requestPool.pRequestDatas[reqIndex].currDownloaded = 0; _requestPool.pRequestDatas[reqIndex].numReqBytes = numReqBytes; .... _requestPool.pRequestDatas->scheduled = true; .... }
PVS-Studio为此段代码发出了两个警告：
- V619数组'_requestPool.pRequestDatas'被用作指向单个对象的指针。 iot_demo_https_s3_download_async.c 973
- V574'_requestPool.pRequestDatas'指针同时用作数组和指向单个对象的指针。 检查行：931、973。iot_demo_https_s3_download_async.c 973
以防万一，让我提醒您：数组名称是指向其第一个元素的指针。 也就是说，如果
_requestPool.pRequestDatas是一个结构数组，则
_requestPool.pRequestDatas [i] .scheduled是对
i数组结构的已
调度成员的评估。如果我们编写
_requestPool.pRequestDatas-> chedule ，结果将是第一个数组结构的成员将被访问。
在上面的代码摘录中，就是这样。 在最后一行中，仅第一个数组结构的成员的值被更改。 就其本身而言，这样的访问已经是可疑的，但是在这种情况下甚至更加清楚：
_requestPool.pRequestDatas数组由整个函数主体中的索引评估。 但是最后，索引操作被遗忘了。
据我了解，最后一行应如下所示：
_requestPool.pRequestDatas[reqIndex].scheduled = true;
下一个错误在于一个小的函数，因此我将对其进行完整介绍：
static BaseType_t prvGGDJsoneq( const char * pcJson, const jsmntok_t * const pxTok, const char * pcString ) { uint32_t ulStringSize = ( uint32_t ) pxTok->end - ( uint32_t ) pxTok->start; BaseType_t xStatus = pdFALSE; if( pxTok->type == JSMN_STRING ) { if( ( uint32_t ) strlen( pcString ) == ulStringSize ) { if( ( int16_t ) strncmp( &pcJson[ pxTok->start ],
PVS-Studio警告： V642 [CWE-197]将'strncmp'函数结果保存在'short'类型变量中是不合适的。 有效位可能会丢失，从而破坏程序的逻辑。 第637章
让我们看一下strncmp函数的定义：
int strncmp( const char *lhs, const char *rhs, size_t count );
在该示例中，大小为32位的
int类型的结果将转换为
int16_t类型的变量。 通过这种“缩小”转换，将丢失返回值的旧位。 例如，如果
strncmp函数返回
0x00010000 ，则在转换过程中单元将丢失，条件将被执行。
看到这种情况真的很奇怪。 如果普通
int可以与零进行比较，为什么在这里需要它？ 另一方面，如果程序员希望此函数有时即使不应该返回
true ，为什么不通过注释支持这种棘手的行为呢？ 但是这种方式有点后门。 无论如何，我倾向于认为这是一个错误。 你觉得呢
未定义的行为和指针
这是一个大例子。 它掩盖了潜在的空指针取消引用：
static void _networkReceiveCallback(....) { IotHttpsReturnCode_t status = IOT_HTTPS_OK; _httpsResponse_t* pCurrentHttpsResponse = NULL; IotLink_t* pQItem = NULL; .... IotMutex_Lock(&(pHttpsConnection->connectionMutex)); pQItem = IotDeQueue_PeekHead(&(pHttpsConnection->respQ)); IotMutex_Unlock(&(pHttpsConnection->connectionMutex)); if (pQItem == NULL) { IotLogError(....); fatalDisconnect = true; status = IOT_HTTPS_NETWORK_ERROR; goto iotCleanup; } .... iotCleanup : if (status != IOT_HTTPS_OK) { if ( pCurrentHttpsResponse->isAsync && pCurrentHttpsResponse->pCallbacks->errorCallback) { pCurrentHttpsResponse->pCallbacks->errorCallback(....); } pCurrentHttpsResponse->syncStatus = status; } .... }
PVS-Studio警告： V522 [CWE-690]可能会取消引用潜在的空指针'pCurrentHttpsResponse'。 iot_https_client.c 1184
最后一个
if块包含有问题的取消引用。 让我们找出这里发生了什么。
该函数以
pCurrentHttpsResponse和
pQItem变量（由
NULL值初始化）和
status变量（由
IOT_HTTPS_OK值）初始化
开始 ，这意味着它们都是正确的。
进一步为
pQItem分配了从
IotDeQueue_PeekHead函数返回的值，该函数返回指向双向链接队列开头的指针。
如果队列为空会怎样？ 在这种情况下，
IotDeQueue_PeekHead函数将返回
NULL： static inline IotLink_t* IotDeQueue_PeekHead (const IotDeQueue_t* const pQueue) { return IotListDouble_PeekHead(pQueue); } .... static inline IotLink_t* IotListDouble_PeekHead (const IotListDouble_t* const pList) { IotLink_t* pHead = NULL; if (pList != NULL) { if (IotListDouble_IsEmpty(pList) == false) { pHead = pList->pNext; } } return pHead; }
此外，条件
pQItem == NULL将变为true，并且控制流将通过
goto传递到函数的下部。 到此时，
pCurrentHttpsResponse指针将保持为空，而
状态将不等于
IOT_HTTPS_OK 。 最后，
如果分支，我们将达到相同的目标，……繁荣！ 好吧，您知道这种取消引用的后果。
好吧 这是一个有点棘手的例子。 现在，我建议您看一下一个非常简单且可以理解的潜在取消引用：
int PKI_mbedTLSSignatureToPkcs11Signature (uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature ) { int xReturn = 0; uint8_t * pxNextLength; uint8_t ucSigComponentLength = pxMbedSignature[ 3 ];
PVS-Studio警告： V595 [CWE-476]在针对nullptr进行验证之前，已使用“ pxMbedSignature”指针。 检查行：52，54。iot_pki_utils.c 52
该函数接收指向
uint8_t的指针 。 都检查两个指针是否为
NULL ，这是一个好习惯-应该立即解决这种情况。
但这是问题所在：选中
pxMbedSignature时 ，它实际上已经在上面的一行中被取消引用。 塔达！
投机代码的另一个示例：
CK_RV vAppendSHA256AlgorithmIdentifierSequence ( uint8_t * x32ByteHashedMessage, uint8_t * x51ByteHashOidBuffer ) { CK_RV xResult = CKR_OK; uint8_t xOidSequence[] = pkcs11STUFF_APPENDED_TO_RSA_SIG; if( ( x32ByteHashedMessage == NULL ) || ( x51ByteHashOidBuffer == NULL ) ) { xResult = CKR_ARGUMENTS_BAD; } memcpy( x51ByteHashOidBuffer, xOidSequence, sizeof( xOidSequence ) ); memcpy( &x51ByteHashOidBuffer[ sizeof( xOidSequence ) ], x32ByteHashedMessage, 32 ); return xResult; }
PVS-Studio警告：- V1004 [CWE-628]在针对nullptr对其进行验证后，不安全地使用了x51ByteHashOidBuffer指针。 检查行：275、280。iot_pkcs11.c 280
- V1004 [CWE-628]在针对nullptr进行验证之后，不安全地使用了x32ByteHashedMessage指针。 检查行：275、281。iot_pkcs11.c 281
分析器警告在检查
NULL后不安全地使用作为指针的函数参数。 实际上，已检查了参数。 但是，如果其中任何一个都不为
NULL ，则除了写入
xResult之外，不会采取任何其他措施
。 代码的这一部分说：“是的，因此论据证明是不好的。 我们现在要记录下来，您-继续前进，继续前进。”
结果：
NULL将被传递给
memcpy。 它能带来什么？ 值将复制到哪里以及哪些值被复制？ 实际上，猜测将无济于事，因为标准
明确指出 ，这样的调用会导致未定义的行为（请参见第1节）。
Amazon FreeRTOS的分析器报告中还有其他不正确的指针处理示例，但我认为，给出的示例足以显示PVS-Studio检测此类错误的功能。 让我们来看看一些新东西。
TRUE！= 1
与模式有关的错误有很多，但不幸的是，这些错误经常被忽略。
事实是，
布尔类型（来自C ++）不同于
布尔类型（通常用于C语言）。 第一个只能包含
true或
false值。 第二个是整数类型（
int ，
long和其他类型）的typedef。
0值为“ false”，其他任何与零不同的值为“ true”。
由于C中没有内置的布尔类型，为方便起见定义了以下常量：
#define FALSE 0 #define TRUE 1
让我们来看一个例子。
int mbedtls_hardware_poll(void* data, unsigned char* output, size_t len, size_t* olen) { int lStatus = MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; HCRYPTPROV hProv = 0; (void)data; if (TRUE == CryptAcquireContextA( &hProv, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) { if (TRUE == CryptGenRandom(hProv, len, output)) { lStatus = 0; *olen = len; } CryptReleaseContext(hProv, 0); } return lStatus; }
PVS-Studio警告：- V676 [CWE-253]比较BOOL类型的变量和TRUE是不正确的。 aws_entropy_hardware_poll.c 48
- V676 [CWE-253]比较BOOL类型的变量和TRUE是不正确的。 正确的表达式是：“ FALSE！= CryptGenRandom（hProv，len，输出）”。 aws_entropy_hardware_poll.c 51
发现错误了吗？ 毫无疑问，它在这里：)
CryptAcquireContextA和
CryptGenRandom函数是
wincrypt.h标头中的标准函数。 如果成功，它们将返回非零值。 让我强调一下，它是
非零的 。 因此，理论上，它可以是不同于零的任何值：
1、314、42、420 。
显然，从示例中编写函数的程序员没有考虑这一点，最后，将结果值与一个进行比较。
TRUE == CryptGenRandom（....）条件
不满足的可能性有
多大 ？ 很难说。 也许，
CryptGenRandom可能比其他值返回1的频率更高，但是也许它可能仅返回1。我们不能肯定地知道这一点：加密功能的实现对凡人程序员是隐藏的：)
重要的是要记住，这样的比较有潜在的危险。 代替：
if (TRUE == GetBOOL())
使用更安全的代码版本：
if (FALSE != GetBOOL())
优化问题
分析仪的一些警告与运行缓慢的结构有关。 例如：
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strstr(contentRangeValStr, "/"); .... }
PVS-Studio警告： V817查找'/'字符而不是字符串更有效。 iot_demo_https_common.c 205
简短而简单，不是吗？ 在这里，
strstr函数仅用于搜索一个字符，该字符作为字符串传入参数中（用双引号引起来）。
可以通过将
strstr替换为
strchr来优化该位置：
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... }
这样，搜索将工作得更快。 一件很小但是很好的事情。
好的，这样的优化是好的，但是分析仪还找到了另一个地方，可以用更加明显的方式对其进行优化：
void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
PVS-Studio警告： V814性能下降。 在循环体内多次调用了“ strlen”函数。 aws_iot_ota_update_demo.c 235
嗯...在循环内部，每次迭代都会调用
strlen ，每次都评估同一行的长度。 不是最有效的操作:)
让我们看一下
clientcredentialIOT_THING_NAME的定义：
#define clientcredentialIOT_THING_NAME ""
要求用户在此处输入其设备的名称。 默认情况下，它是空的，在这种情况下，一切都很好。 如果用户想在此处输入一个长而优美的名称怎么办？ 例如，我很想将自己的
想法称为“
热情而精致的咖啡机BarBarista-N061E Ultimate Edition” 。 您能想象如果我那台漂亮的咖啡机在那之后开始工作慢一点，会给我带来什么惊喜？ 讨厌
要更正错误，值得在体循环之外
大吃一惊 。 毕竟，程序运行期间设备的名称不会更改。 哦，C ++的
constexpr非常适合这里...
好吧，好吧，让我们不要为百合花镀金。 正如我的同事安德烈·卡波夫（Andrey Karpov）所指出的那样，现代编译器知道有什么
奇怪的东西 ，他亲自用二进制代码中的常量监视它们，如果它们知道行的长度不能改变。 因此，很有可能在发行版本构建模式中，将使用预先评估的值来代替实际的行长评估。 但是，这并不总是可行的，因此编写这样的代码不是一个好习惯。
关于MISRA的几句话
PVS-Studio分析仪具有大量规则，可检查您的代码是否符合MISRA C和MISRA C标准。 这些标准是什么？
MISRA是高度负责的嵌入式系统的编码标准。 它包含一组用于编写代码和建立开发过程的严格规则和准则。 这些规则很多，它们不仅旨在消除严重的错误，而且还针对各种“代码味道”。 它还旨在编写最易懂和易读的代码。
因此，遵循MISRA标准不仅有助于避免错误和漏洞，而且还可以大大减少错误和漏洞在已经存在的代码中出现的可能性。
MISRA用于航空，医疗，汽车和军事行业，在这些行业中，人们的生活取决于嵌入式软件的质量。
显然，Amazon FreeRTOS开发人员了解此标准，并且在大多数情况下都遵循它。 这种方法绝对合理：如果您为嵌入式系统编写基础广泛的OS，则必须考虑安全性。
但是，我发现很多违反MISRA标准的行为。 我将不提供“不使用联合”或“功能只能在身体尽头获得回报”之类的规则示例-不幸的是，它们不像大多数MISRA规则那样引人注目。 我想举几个例子，说明可能会导致严重后果的违规行为。
让我们从宏开始：
#define FreeRTOS_ms_to_tick(ms) ( ( ms * configTICK_RATE_HZ + 500 ) / 1000 )
#define SOCKETS_htonl( ulIn ) ( ( uint32_t ) \ ( ( ( ulIn & 0xFF ) << 24 ) | ( ( ulIn & 0xFF00 ) << 8 ) \ | ( ( ulIn & 0xFF0000 ) >> 8 ) | ( ( ulIn & 0xFF000000 ) >> 24 ) ) )
#define LEFT_ROTATE( x, c ) ( ( x << c ) | ( x >> ( 32 - c ) ) )
PVS-Studio警告：- V2546 [MISRA C 20.7]宏及其参数应放在括号中。 考虑检查“ FreeRTOS_ms_to_tick”宏的“ ms”参数。 FreeRTOS_IP.h 201
- V2546 [MISRA C 20.7]宏及其参数应放在括号中。 考虑检查“ SOCKETS_htonl”宏的“ ulIn”参数。 iot_secure_sockets.h 512
- V2546 [MISRA C 20.7]宏及其参数应放在括号中。 考虑检查“ LEFT_ROTATE”宏的参数“ x”，“ c”。 iot_device_metrics.c 90
是的，这正是您的想法。 这些宏的参数未括在方括号中。 如果有人不小心写了类似
val = LEFT_ROTATE(A[i] | 1, B);
这样的宏“调用”将扩展为：
val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );
还记得运营的重点吗？ 首先，进行按位移位，并且只有在此之后才进行按位“或”。 因此，该程序的逻辑将被破坏。 一个简单的例子：如果在
FreeRTOS_ms_to_tick宏中传递表达式“
x + y ”，将会发生什么？ MISRA的主要目标之一就是预防这种情况。
有人可能会争辩说：“如果您有对此一无所知的程序员，那么没有任何标准可以帮助您！” 我不同意。 程序员也是人，无论一个人的经验如何，他们最终都会感到疲倦并犯错。 这就是MISRA强烈建议使用自动分析工具来测试项目符合性的原因之一。
让我向Amazon FreeRTOS的开发人员致辞：PVS-Studio发现了12个更多不安全的宏，因此您需要谨慎对待它们：)
另一个有趣的MISRA违规行为：
static void _responseCompleteCallback(void* pPrivData, IotHttpsResponseHandle_t respHandle, IotHttpsReturnCode_t rc, uint16_t status) { bool* pUploadSuccess = (bool*)pPrivData; if (status == IOT_HTTPS_STATUS_OK) { *pUploadSuccess = true; } else { *pUploadSuccess = false; } IotSemaphore_Post(&(_uploadFinishedSem)); }
您可以自己找到错误吗？
PVS-Studio警告： V2537 [MISRA C 2.7]功能中不应包含未使用的参数。 考虑检查参数：“ rc”。 iot_demo_https_s3_upload_async.c 234
仔细看看：
rc参数未在函数主体中的任何地方使用。 该函数的注释清楚地表明该参数是另一个函数的返回码，并且它可能表示错误。 为什么不以任何方式处理此参数？ 显然这里有些错误。
但是，即使没有这些注释，未使用的参数也经常指向程序的逻辑中断。 否则，为什么在功能签名中需要它们？
在这里，我给出了一个小的函数，该函数对于本文中的示例非常有用。 除此之外，我发现了10个其他未使用的参数。 它们中的许多都用于较大的功能，因此检测起来并不容易。
可疑的是，以前没有找到它们。 毕竟，编译器很容易检测到这种情况。
结论
这些不是分析仪发现的所有问题，但文章原来已经很大。 我希望借此，亚马逊FreeRTOS开发人员将能够纠正一些缺陷，甚至可能希望自己
尝试PVS-Studio 。 这样，彻底调查警告将更加方便。 事实上，使用便捷的界面比查看文本报告要容易得多。
感谢您阅读我们的文章！ 下次见：D
PS恰巧，这篇文章于10月31日发表。万圣节快乐，男孩和女孩！