cryptocup logo

Auditoria CryptoCup

Reading Time: 4 minutes

  •  
  •  
  •  
  •  
  •  
  •  

Introducción

Se le solicitó a CoinFabrik que audite los contratos para el token ERC721 de CryptoCup. En primer lugar, proporcionaremos un resumen de nuestros descubrimientos y, en segundo lugar, mostraremos los detalles de nuestros hallazgos.

Resumen

Los contratos auditados son del repositorio CryptoCup . La auditoría se basa en la confirmación 7b55e9afe2f9818d523e62a89141702c175a8504 y se actualizó para reflejar los cambios en dd0097646b490e82265f5bf5e96ba82875ef3898 .

Los contratos auditados son:

  • AccessControlLayer.sol: Modificadores de llamada de función privilegiada.
  • DataLayer.sol: almacenamiento de contrato.
  • CryptocupDataSource.sol: implementa callbacks Oraclize.
  • CryptocupToken.sol: implementación del token ERC721.
  • GameLogicLayer.sol: lógica de cálculo de puntos.
  • CoreLayer.sol: Funciones adicionales relacionadas con Tokens.
  • DataSourceInterface.sol: Interfaz para CryptocupDataSource.sol.
  • DataSourceCallbackInterface.sol: interfaz de devolución de llamada GameLogic de oraclize.
  • SafeMath.sol: Desbordamiento de operaciones enteras comprobadas.
  • ERC721.sol: interfaz ERC721.

Se realizaron los siguientes análisis:

  • Mal uso de los diferentes métodos de llamada: call.value (), send () y transfer ().
  • Errores de redondeo de enteros, desbordamiento, subdesbordamiento y uso relacionado de las funciones de SafeMath.
  • Viejos pragmas de la versión del compilador.
  • Condiciones de carrera tales como ataques de reentrada o carreras frontales.
  • Mal uso de las marcas de tiempo del bloque, asumiendo que cualquier cosa que no sea estrictamente creciente.
  • Contrato de ataques de softlocking (DoS).
  • Costo de gas potencial de las funciones que están por encima del límite de gas del bloque.
  • Faltan calificadores de funciones y su uso incorrecto.
  • Fallback funciona con un costo de gas más alto que el máximo permitido por transferencia o envío.
  • Código fraudulento o erróneo.
  • Complejidad de la interacción del código y el contrato.
  • Error o falta de manejo de errores.
  • Uso excesivo de transferencias en una sola transacción en lugar de utilizar patrones de retiro.
  • Insuficiente análisis de los requisitos de entrada de la función.

Descubrimientos Detallados

Severidad Crítica

La eliminación del token usa un índice incorrecto en la función de transferencia

En esta función:

La variable tokensOfOwnerMap [fromAddress] es una matriz que contiene los tokens de fromAddress pero esa matriz no es indexable por tokenId, simplemente los almacena. Esto debe solucionarse buscando dicho tokenId en la matriz para obtener el índice adecuado para la eliminación.

Esto se corrigió en commit d299c8bddce19014d6b490cc4d8447305fc3f01a

Severidad Media

La precondición de la retirada de emergencia es contraria a la especificación

Este modificador:

Usado por esta función:

Comprueba lo contrario de lo que se afirma en el comentario, la condición es verdadera desde el momento en que gameFinishedTime se asigna hasta 90 días después de que el juego ha terminado.

La condición debe ser negada, al tiempo que se tiene en cuenta que gameFinishedTime comienza en 0, lo que también hace que la condición sea verdadera antes de que se le asigne.

Esto se corrigió en commit d299c8bddce19014d6b490cc4d8447305fc3f01a

Algunas funciones pueden tener problemas de costo de gas

Actualmente, algunas funciones como esta::

O esta:

Puede tener un alto costo de gas porque es lineal con respecto a la longitud de la matriz. Si los costos de gas son mucho más altos que el límite de bloque promedio, no podrá llamar a estas funciones. Considere poner un límite a estas matrices o limitar el número de elementos que procesa en una sola llamada.

Esto se corrigió en el commit d299c8bddce19014d6b490cc4d8447305fc3f01a y confirma b959f4e68dc16d107551638185fbbb3b924c2f1d

Severidad Mínima

Algunas funciones necesitan documentación

Algunas funciones como la mayoría en GameLogicLayer necesitan la documentación adecuada. Estas son funciones complejas de las cuales no es fácil inferir la intención.

Esto fue abordado por commits múltiples

Advertencias de Solidity

Hay algunas advertencias de solidity irrelevantes que deben corregirse ya que pueden ocultar advertencias más importantes.

Uso de matemática segura no exhaustiva

Algunas integer aritméticos aún no tienen verificaciones de overflow, esto puede ser peligroso ya que los overflows y underflows son relativamente comunes. Considere reemplazar todos los casos con sus operaciones SafeMath equivalentes.

Mejoras

La función getPayoutDistributionId no es necesaria

getPayoutDistributionId se usa para obtener el id de índice correspondiente para la distribución de pagos:

But this is not necessary since setPayoutDistributionId already assigns the corresponding payoutDistribution conditionally. The other two arrays lastPosition and superiorQuota using said id can get the same treatment in the setter function.

Pero esto no es necesario ya que setPayoutDistributionId ya asigna la distribución de pago correspondiente condicionalmente. Los otros dos arrays lastPosition y superiorQuota utilizando dicho id, pueden obtener el mismo tratamiento en la función setter.

Esto se mejoró en commit d299c8bddce19014d6b490cc4d8447305fc3f01a

Conclusión

La idea principal de un token ERC721 es simple y los contratos son simples. Si bien hubo algunos errores críticos, todos tuvieron soluciones simples que fueron abordadas por el equipo de CryptoCup.

Este artículo también esta disponible en inglés


  •  
  •  
  •  
  •  
  •  
  •