Positivos falsos en PVS-Studio: cuán profundo es la madriguera del conejo

Unicorn PVS-Studio y GetNamedSecurityInfo

Nuestro equipo brinda asistencia al cliente rápida y efectiva. Las solicitudes de los usuarios son manejadas únicamente por los programadores, ya que nuestros clientes son programadores y a menudo hacen preguntas difíciles. Hoy les contaré acerca de una solicitud reciente sobre un falso positivo que incluso me obligó a realizar una pequeña investigación para resolver el problema.

Trabajamos duro para reducir al mínimo la cantidad de falsos positivos generados por PVS-Studio. Desafortunadamente, los analizadores estáticos a menudo no pueden distinguir el código correcto de un error porque simplemente no tienen suficiente información. Los falsos positivos son, por lo tanto, inevitables. Sin embargo, no es un problema, ya que puede personalizar fácilmente el analizador para que 9 de cada 10 advertencias apunten a errores genuinos.

Si bien los falsos positivos pueden no parecer un gran problema, nunca dejamos de combatirlos mejorando nuestros diagnósticos. Nuestro equipo capta algunos falsos positivos descarados; otros son reportados por nuestros clientes y usuarios de versiones gratuitas.

Uno de nuestros clientes nos envió recientemente un correo electrónico que decía algo como esto:

Por alguna razón, el analizador dice que cierto puntero siempre es nulo, mientras que no lo es. Además, su comportamiento en un proyecto de prueba es extraño e inestable: a veces emite una advertencia y otras no. Aquí hay un ejemplo sintético que reproduce ese falso positivo:

#include <windows.h> #include <aclapi.h> #include <tchar.h> int main() { PACL pDACL = NULL; PSECURITY_DESCRIPTOR pSD = NULL; ::GetNamedSecurityInfo(_T("ObjectName"), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &pDACL, NULL, &pSD); auto test = pDACL == NULL; // V547 Expression 'pDACL == 0' is always true. return 0; } 

No es difícil adivinar cómo nuestros usuarios ven falsos positivos como ese. La función GetNamedSecurityInfo obviamente modifica el valor de la variable pDACL . ¿Qué evitó que los desarrolladores hicieran un controlador para casos simples como ese? ¿Y por qué no se emite la advertencia en cada sesión? ¿Quizás es un error en el analizador en sí mismo, por ejemplo, una variable no inicializada?

Por desgracia ... Apoyar a los usuarios de un analizador de código estático no es un trabajo fácil, pero fue mi propia elección hacerlo. Entonces, me arremangué y empecé a investigar el problema.

Comencé comprobando la descripción de la función GetNamedSecurityInfo y asegurándome de que su llamada realmente implicara modificar el valor de la variable pDACL . Aquí está la descripción del sexto argumento:
ppdacl

Un puntero a una variable que recibe un puntero a la DACL en el descriptor de seguridad devuelto o NULL si el descriptor de seguridad no tiene DACL. El puntero devuelto es válido solo si establece el indicador DACL_SECURITY_INFORMATION. Además, este parámetro puede ser NULL si no necesita la DACL.

Sé que PVS-Studio obviamente debería ser capaz de manejar un código tan simple sin generar una advertencia falsa. En ese momento, mi intuición ya me decía que el caso no era trivial y que me iba a llevar bastante tiempo resolverlo.

Mis dudas se confirmaron cuando no pude reproducir el falso positivo con nuestra versión alfa actual del analizador o con la versión instalada en la computadora del usuario. No importa lo que hice, el analizador guardó silencio.

Le pedí al cliente que me enviara el archivo i preprocesado generado para el programa de ejemplo. Él hizo eso y yo seguí con mi investigación.

El analizador produjo el falso positivo en ese archivo de inmediato. Por un lado, fue bueno que finalmente hubiera logrado reproducirlo. Por otro lado, tuve la sensación de que esta imagen podría ilustrarse mejor:

wat


¿Por qué este sentimiento? Verá, sé perfectamente cómo funcionan tanto el analizador como el diagnóstico V547 . ¡Simplemente no hay forma de que puedan generar un falso positivo como ese, nunca!

Bien, hagamos un poco de té y continuemos.

La llamada a la función GetNamedSecurityInfo se expande en el siguiente código:

 ::GetNamedSecurityInfoW(L"ObjectName", SE_FILE_OBJECT, (0x00000004L), 0, 0, &pDACL, 0, &pSD); 

Este código se ve igual tanto en el archivo i preprocesado en mi computadora como en el archivo enviado por el usuario.

Hmm ... OK, veamos la declaración de esta función. Esto es lo que tengo en mi archivo:

 __declspec(dllimport) DWORD __stdcall GetNamedSecurityInfoW( LPCWSTR pObjectName, SE_OBJECT_TYPE ObjectType, SECURITY_INFORMATION SecurityInfo, PSID * ppsidOwner, PSID * ppsidGroup, PACL * ppDacl, PACL * ppSacl, PSECURITY_DESCRIPTOR * ppSecurityDescriptor ); 

Todo es lógico y claro. Nada inusual

Luego miro el archivo del usuario y ...

wat wat wat


Lo que veo allí no pertenece a nuestra realidad:

 __declspec(dllimport) DWORD __stdcall GetNamedSecurityInfoW( LPCWSTR pObjectName, SE_OBJECT_TYPE ObjectType, SECURITY_INFORMATION SecurityInfo, const PSID * ppsidOwner, const PSID * ppsidGroup, const PACL * ppDacl, const PACL * ppSacl, PSECURITY_DESCRIPTOR * ppSecurityDescriptor ); 

Tenga en cuenta que el parámetro formal pp Dacl está marcado como const .

Wat? WTF? Wat? WTF?

¿Qué es esa constante ? ¿Qué está haciendo aquí?

Bueno, al menos sé con certeza que el analizador es inocente y puedo defender su honor.

El argumento es un puntero a un objeto constante. Resulta que, desde el punto de vista del analizador, la función GetNamedSecurityInfoW no puede modificar el objeto al que hace referencia el puntero. Por lo tanto, en el siguiente código:

 PACL pDACL = NULL; PSECURITY_DESCRIPTOR pSD = NULL; ::GetNamedSecurityInfo(_T("ObjectName"), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &pDACL, NULL, &pSD); auto test = pDACL == NULL; // V547 Expression 'pDACL == 0' is always true. 

la variable pDACL no puede cambiar, de lo cual el analizador nos advierte correctamente (la expresión 'pDACL == 0' siempre es verdadera).

Bien, ahora sabemos qué desencadena la advertencia. Lo que aún no sabemos es de dónde vino esa palabra clave constante . ¡Simplemente no puede estar allí!

Bueno, tengo una suposición, y se confirma por lo que encuentro en Internet. Resulta que hay una versión antigua del archivo aclapi.h con una descripción de función incorrecta. También me he encontrado con un par de enlaces interesantes:


Entonces, había una vez una descripción de la función en el archivo aclapi.h (6.0.6002.18005-Windows 6.0):

 WINADVAPI DWORD WINAPI GetNamedSecurityInfoW( __in LPWSTR pObjectName, __in SE_OBJECT_TYPE ObjectType, __in SECURITY_INFORMATION SecurityInfo, __out_opt PSID * ppsidOwner, __out_opt PSID * ppsidGroup, __out_opt PACL * ppDacl, __out_opt PACL * ppSacl, __out_opt PSECURITY_DESCRIPTOR * ppSecurityDescriptor ); 

Luego, alguien cambió el tipo del parámetro pObjectName pero desordenó los tipos de punteros en el camino al agregar la palabra clave const . Y el archivo aclapi.h (6.1.7601.23418-Windows 7.0) terminó de la siguiente manera:

 WINADVAPI DWORD WINAPI GetNamedSecurityInfoW( __in LPCWSTR pObjectName, __in SE_OBJECT_TYPE ObjectType, __in SECURITY_INFORMATION SecurityInfo, __out_opt const PSID * ppsidOwner, __out_opt const PSID * ppsidGroup, __out_opt const PACL * ppDacl, __out_opt const PACL * ppSacl, __out PSECURITY_DESCRIPTOR * ppSecurityDescriptor ); 

diff 1


Ahora estaba claro que nuestro usuario había estado trabajando con esa versión muy incorrecta de aclapi.h, que luego confirmó en su correo electrónico. No pude reproducir el error porque estaba usando una versión más reciente.

Así es como se ve la descripción de la función fija en el último archivo aclapi.h (6.3.9600.17415-Windows_8.1).

 WINADVAPI DWORD WINAPI GetNamedSecurityInfoW( _In_ LPCWSTR pObjectName, _In_ SE_OBJECT_TYPE ObjectType, _In_ SECURITY_INFORMATION SecurityInfo, _Out_opt_ PSID * ppsidOwner, _Out_opt_ PSID * ppsidGroup, _Out_opt_ PACL * ppDacl, _Out_opt_ PACL * ppSacl, _Out_ PSECURITY_DESCRIPTOR * ppSecurityDescriptor ); 

diff 2


El tipo del argumento pObjectName sigue siendo el mismo, pero los const adicionales se han ido. Todo está bien de nuevo, pero todavía hay encabezados rotos en uso en algún lugar.

Le explico todo eso al cliente, y él está feliz de ver resuelto el problema. Además, descubrió por qué el falso positivo no ocurría regularmente:

Ahora recuerdo haber experimentado con conjuntos de herramientas en este proyecto de prueba hace algún tiempo. La configuración de depuración se configuró en Platform Toolset de manera predeterminada para Visual Studio 2017 - "Visual Studio 2017 (v141)", mientras que la configuración de Release se configuró en "Visual Studio 2015 - Windows XP (v140_xp)". Ayer simplemente estaba cambiando entre las configuraciones, y la advertencia aparecería y desaparecería en consecuencia.

Eso es todo La investigación ha terminado. Discutimos el problema con el cliente y decidimos no agregar ningún error al analizador para que pueda manejar este error del archivo de encabezado. Lo más importante es que hemos resuelto el problema. "Caso desestimado", como dicen.

Conclusión

PVS-Studio es un producto de software complejo, que recopila grandes cantidades de información del código de los programas y lo utiliza en diversas técnicas de análisis . En este caso particular, resultó ser demasiado inteligente, terminando con un falso positivo debido a una descripción de función incorrecta.

Conviértase en nuestros clientes, y tiene la garantía de obtener un rápido apoyo profesional de mí y mis compañeros de equipo.

Source: https://habr.com/ru/post/441124/


All Articles