Ya han pasado tres meses desde que terminó 2018. Para muchos, voló casi imperceptiblemente, pero para nosotros, los desarrolladores de PVS-Studio, resultó estar muy saturado. Trabajamos duro, luchamos sin miedo por el avance del análisis estático a las masas y buscamos nuevos errores en proyectos de código abierto escritos en C, C ++, C # y Java. ¡Los diez más interesantes que hemos recopilado para ti en este artículo!
Buscamos lugares interesantes utilizando el analizador de código estático
PVS-Studio . Puede detectar errores y vulnerabilidades potenciales en el código escrito en los idiomas mencionados anteriormente.
Si está interesado en buscar errores usted mismo, siempre puede descargar y probar nuestro analizador. Proporcionamos una
versión gratuita del analizador para estudiantes y programadores entusiastas, una
licencia gratuita para desarrolladores de proyectos de código abierto, así como una
versión de prueba para todo. Quién sabe, ¿quizás para el próximo año puedas estar entre los 10 mejores? :)
Nota: sugiero que el lector se revise a sí mismo y, antes de mirar la advertencia del analizador, intente identificar las anomalías él mismo. ¿Cuántos errores puedes encontrar?
Décimo lugar
Fuente:
Y nuevamente al espacio: cómo visitó el unicornio StellariumEste error fue descubierto al verificar el planetario virtual Stellarium.
El fragmento de código anterior, aunque pequeño, está plagado de un error bastante complicado:
Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3) : distance(0.0f), sDistance(0.0f) { Plane(v1, v2, v3, SPolygon::CCW); }
Has encontrado
Advertencia de PVS-Studio: V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> Plane :: Plane (....)'. Plane.cpp 29
El autor del código quería inicializar parte de los campos del objeto utilizando otro constructor anidado en el principal. Es cierto, en cambio, solo logró crear un objeto temporal que sería destruido al abandonar su área de visibilidad. Por lo tanto, varios campos del objeto permanecerán sin inicializar.
En lugar de una llamada de constructor anidada, debe usar el constructor delegante introducido en C ++ 11. Por ejemplo, podrías hacer esto:
Plane::Plane(Vec3f& v1, Vec3f& v2, Vec3f& v3) : Plane(v1, v2, v3, SPolygon::CCW) { distance = 0.0f; sDistance = 0.0f; }
Entonces todos los campos requeridos se inicializarán correctamente. ¿No es maravilloso?
Noveno lugar
Fuente:
Perl 5: Cómo se ocultan los errores de macroEn noveno lugar hace alarde de una macro notable del código fuente de Perl 5.
Al recopilar errores para escribir el artículo, mi colega Svyatoslav se encontró con una advertencia emitida por el analizador sobre el uso de una macro. Aquí esta:
PP(pp_match) { .... MgBYTEPOS_set(mg, TARG, truebase, RXp_OFFS(prog)[0].end); .... }
Para averiguar cuál era el problema, Svyatoslav profundizó. Abrió la definición de macro y vio que contenía varias macros anidadas más, algunas de las cuales también tenían macros anidadas. Fue tan difícil de entender que tuve que usar un archivo preprocesado. Pero, por desgracia, esto no ayudó. En lugar de la línea de código anterior, Svyatoslav descubrió esto:
(((targ)->sv_flags & 0x00000400) && (!((targ)->sv_flags & 0x00200000) || S_sv_only_taint_gmagic(targ)) ? (mg)->mg_len = ((prog->offs)[0].end), (mg)->mg_flags |= 0x40 : ((mg)->mg_len = (((targ)->sv_flags & 0x20000000) && !__builtin_expect(((((PL_curcop)->cop_hints + 0) & 0x00000008) ? (_Bool)1 :(_Bool)0),(0))) ? (ssize_t)Perl_utf8_length( (U8 *)(truebase), (U8 *)(truebase)+((prog->offs)[0].end)) : (ssize_t)((prog->offs)[0].end), (mg)->mg_flags &= ~0x40));
Advertencia PVS-Studio: V502 Quizás el operador '?:' Funciona de una manera diferente a la esperada. El operador '?:' Tiene una prioridad menor que el operador '&&'. pp_hot.c 3036
Creo que será difícil encontrar tal error con mis ojos. Honestamente, meditamos en este código durante mucho tiempo y llegamos a la conclusión de que, de hecho, no hay ningún error aquí. Pero en cualquier caso, este es un ejemplo bastante entretenido de código ilegible.
Se dice que las macros son malvadas. Por supuesto, hay momentos en que resultan ser indispensables, pero si puede reemplazar la macro con una función, definitivamente debería hacerlo.
Las macros anidadas son especialmente pesadas. No solo porque son difíciles de entender, sino también porque pueden dar resultados impredecibles. Si el autor de una macro accidentalmente comete un error en dicha macro, será mucho más difícil encontrarla que en una función.
Octavo lugar
Fuente:
cromo: otros erroresEl siguiente ejemplo fue tomado de una serie de artículos sobre el análisis del proyecto Chromium. Se cubrió en la biblioteca WebRTC.
std::vector<SdpVideoFormat> StereoDecoderFactory::GetSupportedFormats() const { std::vector<SdpVideoFormat> formats = ....; for (const auto& format : formats) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } } return formats; }
Advertencia de PVS-Studio: V789 CWE-672 Los iteradores para el contenedor de 'formatos', utilizados en el bucle basado en rango, se vuelven inválidos cuando se llama a la función 'push_back'. stereocodecfactory.cc 89
El error es que el tamaño del vector de
formatos cambia dentro del bucle basado en rango. Los bucles basados en el rango se basan en iteradores, por lo que cambiar el tamaño del contenedor dentro de dichos bucles puede provocar la invalidación de estos iteradores.
Este error persistirá si reescribe el bucle utilizando iteradores explícitos. Por lo tanto, para mayor claridad, puede traer este código:
for (auto format = begin(formats), __end = end(formats); format != __end; ++format) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } }
Por ejemplo, cuando se usa el método
push_back , se puede liberar un vector, y luego los iteradores apuntarán a un área de memoria no válida.
Para evitar tales errores, debe cumplir con la regla: nunca cambie el tamaño del contenedor dentro del bucle, cuyas condiciones están vinculadas a este contenedor. Esto se aplica a bucles basados en rango y bucles que utilizan iteradores. Puede leer sobre qué operaciones pueden conducir a la invalidación de iteradores en las
discusiones sobre StackOverflow.
Séptimo lugar
Fuente:
Godot: uso regular de analizadores de código estáticoEl primer ejemplo de la industria de los videojuegos será un fragmento de código que descubrimos en el motor de juegos Godot. Puede que tenga que sudar para descubrir el error con sus ojos, pero estoy seguro de que nuestros lectores sofisticados pueden manejarlo:
void AnimationNodeBlendSpace1D::add_blend_point( const Ref<AnimationRootNode> &p_node, float p_position, int p_at_index) { ERR_FAIL_COND(blend_points_used >= MAX_BLEND_POINTS); ERR_FAIL_COND(p_node.is_null()); ERR_FAIL_COND(p_at_index < -1 || p_at_index > blend_points_used); if (p_at_index == -1 || p_at_index == blend_points_used) { p_at_index = blend_points_used; } else { for (int i = blend_points_used - 1; i > p_at_index; i++) { blend_points[i] = blend_points[i - 1]; } } .... }
Advertencia de PVS-Studio: V621 CWE-835 Considere inspeccionar el operador 'para'. Es posible que el bucle se ejecute incorrectamente o no se ejecute en absoluto. animation_blend_space_1d.cpp 113
Consideremos la condición del ciclo con más detalle. La variable de contador se inicializa con el valor
blend_points_used - 1 . Al mismo tiempo, en función de las dos comprobaciones anteriores (en
ERR_FAIL_COND y en
if ), queda claro que en el momento en que se ejecuta el bucle,
blend_points_used siempre será mayor que
p_at_index . Por lo tanto, la condición del bucle siempre será verdadera o el bucle no se ejecutará en absoluto.
Si
blend_points_used es 1 == p_at_index , entonces el ciclo no se ejecuta.
En todos los demás casos, la comprobación
i> p_at_index siempre será verdadera, ya que el contador
i aumenta en cada iteración del bucle.
Puede parecer que el ciclo se ejecutará para siempre, pero no lo es.
Primero, habrá un desbordamiento de enteros de la variable
i , que es un comportamiento indefinido. Por lo tanto, confiar en esto no vale la pena.
Si fuera de tipo
unsigned int , luego de que el contador alcance el valor máximo posible, el operador
i ++ lo convertiría en
0 . Este comportamiento está definido por el estándar y se denomina "ajuste sin firmar". Sin embargo, debe tener en cuenta que el uso de dicho mecanismo tampoco
es una buena idea .
Eso fue primero, ¡pero todavía hay segundo! El hecho es que ni siquiera alcanzará el desbordamiento de enteros. Donde antes la matriz se va al extranjero. Esto significa que se intentará acceder al área de memoria fuera del bloque asignado para la matriz. Y esto también es un comportamiento vago. Ejemplo clásico :)
Para que sea más fácil evitar tales errores, solo puedo dar un par de recomendaciones:
- Escriba código más simple e intuitivo
- Haga una revisión de código más exhaustiva y escriba más pruebas para el código recién escrito
- Use analizadores estáticos;)
Sexto lugar
Fuente:
Amazon Lumberyard: The Cry of the SoulOtro ejemplo de la industria de gamedev, es decir, del código fuente del motor AAA de Amazon Lumberyard.
void TranslateVariableNameByOperandType(....) {
Advertencia de PVS-Studio: V523 La declaración 'then' es equivalente a la declaración 'else'. toglsloperand.c 700
Amazon Lumberyard se está desarrollando como un motor multiplataforma. Por lo tanto, los desarrolladores están tratando de soportar tantos compiladores como sea posible. El programador Igor se topó con el compilador Qualcomm, como nos dicen los comentarios.
No se sabe si Igor pudo completar su tarea y hacer frente a los controles "paranoicos" del compilador, pero dejó un código muy extraño. Es extraño que tanto las ramas de la
declaración if como las demás contengan un código absolutamente idéntico. Lo más probable es que dicho error se haya producido como resultado de un descuidado Copy-Paste.
Ni siquiera sé qué se puede aconsejar aquí. Por lo tanto, solo deseo que los desarrolladores de Amazon Lumberyard tengan éxito en la corrección de errores, ¡y buena suerte para el programador Igor!
Quinto lugar
Fuente: una
vez más, el analizador PVS-Studio resultó ser más atento que una personaUna historia interesante sucedió con el siguiente ejemplo. Mi colega Andrei Karpov estaba preparando un artículo sobre la próxima prueba del marco Qt. En el curso de escribir errores notables, se encontró con una advertencia del analizador, que consideró falsa. Aquí está el fragmento de código relevante y la advertencia:
QWindowsCursor::CursorState QWindowsCursor::cursorState() { enum { cursorShowing = 0x1, cursorSuppressed = 0x2 }; CURSORINFO cursorInfo; cursorInfo.cbSize = sizeof(CURSORINFO); if (GetCursorInfo(&cursorInfo)) { if (cursorInfo.flags & CursorShowing)
Advertencia de PVS-Studio: V616 CWE-480 La constante con nombre 'CursorShowing' con el valor de 0 se utiliza en la operación bit a bit. qwindowscursor.cpp 669
Es decir, PVS-Studio juró en un lugar donde, obviamente, ¡no hubo error! No puede ser que la constante
CursorShowing sea
0 , porque literalmente un par de líneas encima se inicializan a
1 .
Como se usó una versión inestable del analizador para la verificación, Andrei dudó de la exactitud de la advertencia. Examinó cuidadosamente esta sección de código varias veces, y aún no encontró un error. Como resultado, escribió este falso positivo al rastreador de errores para que otros colegas pudieran corregir la situación.
Y solo con un análisis detallado quedó claro que PVS-Studio volvió a estar más atento que una persona. El valor
0x1 se asigna a la constante nombrada
cursorShowing , y la operación constante de bit "y" involucra la constante nombrada
CursorShowing . Estas son constantes completamente diferentes, porque la primera comienza con una letra minúscula y la segunda con una letra mayúscula.
El código se compila correctamente, porque la clase
QWindowsCursor realmente contiene una constante con ese nombre. Aquí está su definición:
class QWindowsCursor : public QPlatformCursor { public: enum CursorState { CursorShowing, CursorHidden, CursorSuppressed }; .... }
Si no asigna explícitamente una constante enum con nombre, se inicializará de manera predeterminada. Como
CursorShowing es el primer elemento de una enumeración, se establecerá en
0 .
Para evitar tales errores, no debe dar a las entidades nombres demasiado similares. Debe seguir especialmente esta regla con mucho cuidado si estas entidades son del mismo tipo o se pueden lanzar implícitamente entre sí. De hecho, en tales casos será casi imposible detectar un error a simple vista, y el código incorrecto se compilará con éxito y vivirá felizmente dentro de su proyecto.
Cuarto lugar
Fuente:
disparamos en el pie, procesando los datos de entradaNos estamos acercando a los tres primeros finalistas, y el error del proyecto FreeSWITCH es el siguiente.
static const char *basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if (fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } command_buf[strlen(command_buf)-1] = '\0'; break; } .... }
Advertencia de PVS-Studio: V1010 CWE-20 Los datos contaminados no
verificados se usan en el índice: 'strlen (command_buf)'.
El analizador advierte que la expresión
strlen (command_buf) - 1 usa datos no verificados. Y realmente: si
command_buf resulta estar vacío desde el punto de vista de la cadena de lenguaje C (que contiene un solo carácter - '\ 0'), entonces
strlen (command_buf) devolverá
0 . En este caso, se
llamará a command_buf [-1] , que representa un comportamiento indefinido. Problemas
La esencia de este error no es ni siquiera
por qué sucede, sino
cómo sucede. Este error es uno de esos ejemplos agradables que puede "tocar" por su cuenta, reproducir. Puede iniciar FreeSwitch, realizar algunas acciones que conducirán a la ejecución de la sección de código anterior y pasar al programa una línea vacía para la entrada.
Como resultado, con un movimiento de muñeca, un programa de trabajo convierte (no, no
pantalones cortos elegantes ) en uno que no funciona. Los detalles sobre cómo reproducir este error se pueden encontrar en el artículo fuente en el enlace de arriba, pero por ahora daré un resultado claro:
Recuerde que la entrada puede ser cualquier cosa, y siempre debe verificarla. Entonces el analizador no jurará, y el programa será más confiable.
Ahora es el momento de lidiar con nuestros ganadores: ¡nos estamos moviendo a la final!
Tercer lugar
Fuente:
NCBI Genome Workbench: Endangered ResearchLos tres ganadores son abiertos por un código del proyecto NCBI Genome Workbench, un conjunto de herramientas para estudiar y analizar datos genéticos. Aunque no es necesario ser un superhombre genéticamente modificado para encontrar un error aquí, muchos son conscientes de esta posibilidad.
void tds_answer_challenge(....) { .... if (ntlm_v == 1) { .... memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); ... } else { .... } }
Advertencias de PVS-Studio:- V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'hash'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 365
- V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'passwd_buf'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 366
¿Lograste encontrar un error? Si es así, entonces ... ¡bien hecho! ... bueno, o aún un superhombre genéticamente modificado.
El hecho es que los compiladores de optimización modernos pueden hacer mucho para que el programa ensamblado funcione más rápido. En particular, los compiladores pueden rastrear que el búfer pasado a
memset no se usa en ningún otro lugar.
En este caso, pueden eliminar la llamada de
memset "innecesaria" y tienen todo el derecho de hacerlo. Entonces, un búfer que almacena datos importantes puede permanecer en la memoria para el deleite de los atacantes.
En este contexto, el comentario literario "con seguridad es bueno ser pedante" parece aún más divertido. A juzgar por el muy pequeño número de advertencias emitidas para este proyecto, los desarrolladores se esforzaron mucho por tener cuidado y escribir un código seguro. Sin embargo, como podemos ver, omitir esta falla de seguridad es muy simple. De acuerdo con la Enumeración de Debilidad Común, un defecto se clasifica como
CWE-14 : Eliminación del Código por parte del Compilador para Borrar Buffers.
Para borrar la limpieza de memoria, use la función
memset_s () . No solo es más seguro que
memset () , sino que tampoco puede ser "ignorado" por el compilador.
Segundo lugar
Fuente:
cómo PVS-Studio resultó ser más atento que tres programadores y medioEl medallista de plata de este top nos lo envió uno de nuestros clientes. Estaba seguro de que el analizador generaba falsas advertencias.
Eugene recibió la carta, la escaneó brevemente y se la envió a Svyatoslav. Svyatoslav miró pensativamente la sección de código enviada por el cliente y pensó: "¿Puede el analizador estar tan descaradamente equivocado?" Por lo tanto, fue a consultar con Andrei. También revisó el sitio y decidió: de hecho, el analizador da falsos positivos.
¿Qué puedes hacer? Necesitas arreglarlo. Y solo cuando Svyatoslav comenzó a hacer ejemplos sintéticos para formalizar la tarea como rastreador de errores, se dio cuenta de lo que estaba sucediendo.
De hecho, hubo errores en el código, pero ninguno de los programadores pudo detectarlos. Honestamente, el autor de este artículo tampoco tuvo éxito.
¡Y esto a pesar del hecho de que el analizador claramente emitió advertencias para los lugares equivocados!
¿Puedes encontrar un error tan astuto? Ponte a prueba para la vigilancia y la atención.
Advertencia de PVS-Studio:- V560 Una parte de la expresión condicional siempre es falsa: (ch> = 0x0FF21). decodew.cpp 525
- V560 Una parte de la expresión condicional siempre es verdadera: (ch <= 0x0FF3A). decodew.cpp 525
- V560 Una parte de la expresión condicional siempre es falsa: (ch> = 0x0FF41). decodew.cpp 525
- V560 Una parte de la expresión condicional siempre es verdadera: (ch <= 0x0FF5A). decodew.cpp 525
Si tiene éxito, ¡no tendrá mi respeto!
El error radica en el hecho de que el operador de negación lógica (!) No se aplica a toda la condición, sino solo a su primera subexpresión:
!((ch >= 0x0FF10) && (ch <= 0x0FF19))
Si se cumple esta condición, el valor de la variable
ch se encuentra en el intervalo [0x0FF10 ... 0x0FF19]. Por lo tanto, las cuatro comparaciones adicionales ya no tienen sentido: siempre serán verdaderas o falsas.
Para evitar tales errores, vale la pena seguir algunas reglas. Primero, es muy conveniente y claro alinear el código con una tabla. En segundo lugar, no sobrecargue las expresiones con paréntesis. Por ejemplo, este código se puede reescribir así:
const bool isLetterOrDigit = (ch >= 0x0FF10 && ch <= 0x0FF19)
Luego, en primer lugar, el número de paréntesis se vuelve mucho más pequeño y, en segundo lugar, aumenta la probabilidad de "atrapar" un error cometido por los ojos.
Y ahora, cereza: ¡nos estamos moviendo al primer lugar!
Primer lugar
Fuente:
System in shock: errores interesantes en los códigos fuente del legendario System Shock¡Entonces, el finalista de nuestro top de hoy es un error del legendario System Shock! Este juego, lanzado en 1994, se convirtió en el progenitor e inspirador de juegos icónicos como Dead Space, BioShock y Deus Ex.
Pero primero, tengo que admitir algo. Lo que le mostraré ahora no contiene ningún error. En general, ni siquiera es un fragmento de código, ¡pero no pude resistirme a no compartir esto con ustedes!
El hecho es que en el proceso de analizar el código fuente del juego, mi colega Victoria encontró muchos comentarios interesantes. Aquí y allá, de repente, hubo bromas y comentarios irónicos, e incluso versos:
Para nuestros lectores de habla rusa, hice una traducción gratuita aproximada:
Los desarrolladores del juego dejaron estos comentarios a principios de los noventa ... Por cierto, Doug Church, el diseñador jefe de System Shock, también estaba escribiendo código. Quién sabe, ¿quizás alguno de estos comentarios fue escrito por él personalmente? Espero que sobre los hombres con toallas, este no sea asunto suyo :)Conclusión
En conclusión, quiero agradecer a mis colegas por buscar nuevos errores y escribir artículos sobre ellos. Gracias chicos! Sin ti, este artículo no hubiera resultado tan interesante.También quiero hablar un poco sobre nuestros logros, porque durante todo un año hemos estado involucrados en algo más que solo buscar errores. También desarrollamos y mejoramos el analizador, como resultado de lo cual sufrió cambios significativos.Por ejemplo, agregamos soporte para varios compiladores nuevos y ampliamos la lista de reglas de diagnóstico. También proporcionamos soporte inicial para los estándares MISRA C y MISRA C ++ . La innovación más importante y lenta fue el soporte de un nuevo lenguaje. ¡Sí, ahora podemos analizar el código Java ! Y hemos actualizado el ícono:)También quiero agradecer a nuestros lectores. ¡Gracias por leer nuestros artículos y escribirnos! Sus comentarios son muy agradables e importantes para nosotros.En esto, nuestros 10 errores principales de C ++ para el año 2018 llegaron a su fin. ¿Qué lugares te gustaron más y por qué? ¿Encontraste ejemplos interesantes en 2018? ¡Cuéntanoslo en los comentarios!¡Hasta la próxima!
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: George Gribkov. Los 10 errores principales de los proyectos de C ++ encontrados en 2018