6 de diciembre de 2021
Introducción
El Cuenta El equipo nos pidió que revisáramos y auditáramos un conjunto de contratos con el objetivo final de mejorar los contratos de gobierno común y darles más flexibilidad. Miramos el código y ahora publicamos nuestros resultados.
Resumen del sistema
El sistema se basa en tres contratos principales:
- A
SafeGuard
modelo de contrato Este contrato es eladmin
menosTimelock
contrato, y agrega una capa de roles modulares sobre elTimelock
las acciones de Este contrato define varios roles que tienen responsabilidad y acceso por separado sobre el estado de una propuesta (puesta en cola, cancelación y ejecución). - A
SafeGuardFactory
contrato despliega un nuevoSafeGuard
y una nueva correspondienteTimelock
contrato. Luego establece la dirección de bloqueo de tiempo dentro delSafeGuard
contrato y registra el nuevoSafeGuard
en elRegistry
. - A
Registry
que contiene una lista de desplegadosSafeGuards
con su correspondiente número de versión.
El SafeGuardFactory
básicamente generará un nueva SafeGuard
cada vez que se llama. A SafeGuard
es un envoltorio Timelock
operaciones que añade roles diferentes y separados para cada acción. Los roles se estructuran mediante el uso de OpenZeppelin's AccessControlEnumerable
contrato dando más flexibilidad y compatibilidad a las billeteras multisig y casos de uso comercial donde varias direcciones pueden tener el mismo rol compartido.
Actualizar: En PR # 10, el equipo de Tally ha decidido eliminar el Registry
contrato. La lista de salvaguardas implementadas ahora se almacena dentro del SafeGuardFactory
contrato, en el safeGuards
conjunto enumerable, junto con su versión almacenada en el safeGuardVersion
cartografía.
Roles
El SafeGuard
contrato define los siguientes roles:
Actualizar: El CREATOR_ROLE
El rol se eliminó como una solución para el problema L02.
Lo que hacemos
Compromiso auditado b2c63a9dfc4090be13320d999e7c6c1d842625d3
de las safeguard
repositorio. En el alcance están los contratos inteligentes en el contracts
directorio. sin embargo, el mocks
El directorio se consideró fuera del alcance.
Supuestos
El sistema no está destinado a ser actualizable. los Registry
la dirección está establecida en el constructor de las SafeGuardFactory
y cada SafeGuard
puede tener el Timelock
configurar solo una vez. Esto significa que si un nuevo Registry
se despliega un nuevo SafeGuardFactory
también debe desplegarse. Si el Timelock
cambios de implementación, el antiguo SafeGuard
s quedarán obsoletos y habrá que implementar otros nuevos.
Además, el sistema depende en gran medida de la aplicación de un Timelock
contrato que se consideró fuera del alcance de esta auditoría. El equipo aún no ha finalizado la implementación del Timelock
contrato. En el momento de redactar este informe, la implementación del Compound de timelock se está utilizando en el proyecto.
El código base ha sido auditado por dos auditores durante el transcurso de una semana y aquí presentamos nuestros hallazgos.
Severidad crítica
Ninguna.
Alta severidad
[H01] ETH se puede bloquear dentro del Timelock
contrato
El Tally
El equipo originalmente basó sus implementaciones en el terreno de la GovernorBravoDelegate
contrato compuesto.
Durante el transcurso de esta auditoría, el Tally
equipo descubierto una limitación en el gobernador de Compound donde ETH envió directamente al Timelock
no está disponible para que lo usen las propuestas de gobernanza y, aunque no está atascado de forma permanente, requiere una solución elaborada para recuperarlo.
Esto se debe a que la implementación del gobernador requiere que todo el valor de una propuesta se adjunte como msg.value
por la cuenta que desencadena la ejecución, no utilizando de ninguna manera la Timelock
fondos ETH.
El El mismo problema se identificó más tarde en el SafeGuard
implementación y el equipo es consciente del problema y está en proceso de solucionarlo.
Mientras soluciona el problema, considere usar el enfoque adoptado por la biblioteca OpenZeppelin para el mismo problema.
Actualizar: Corregido en compromiso 7337db227edda83533be586135d96ddac4f5bf29
.
[H02] SafeGuardFactory se puede congelar
El Registry
contrato tiene por objeto realizar un seguimiento de todos los SafeGuards
que el SafeGuardFactory
produce. tiene el exterior register
función que se utiliza para este fin.
Al mismo tiempo, el SafeGuardFactory
tiene, en su constructor, la asignación del local registry
valor al valor de entrada. No hay posibilidad de cambiar el valor de la registry
variable y por ello, si una nueva Registry
se implementa, también se debe implementar una nueva fábrica.
El SafeGuardFactory
tiene la createSafeGuard
función, a cargo de la primera implementando un nuevo SafeGuard
, entonces un nuevo Timelock
con la dirección del SafeGuard
as admin
, luego configurando el timelock
variable de la SafeGuard
contrato y finalmente registrando el SafeGuard
en el registro.
El tema es que cualquier llamada a createSafeGuard
puede ser forzado a fallar por un atacante que puede registrar directamente la dirección determinista del nuevo SafeGuard
antes de su creación. Siempre que un contrato crea un new
ejemplo, su nonce aumenta, y la dirección donde se implementaría la nueva instancia del contrato se puede determinar mediante la dirección del contrato original y su nonce. Por lo tanto, un atacante puede precalcular muchas de las direcciones donde el nuevo SafeGuards
se implementará y registrará esas direcciones en el Registry
llamando al register
función. Esto daría lugar a las llamadas a createSafeGuard
a revertir ya que el Registry
ya contiene la dirección.
Para evitar que actores externos llamen públicamente la Register
contrato, considere restringir el acceso a la register
función para aceptar llamadas exclusivamente por el SafeGuardFactory
.
Actualizar: Fijado en PR # 10. El equipo de Tally ha eliminado el Registry
contrato.
Severidad media
Ninguna.
Gravedad baja
[L01] Código comentado
El Registry
el contrato incluye una línea de código comentada. Para mejorar la legibilidad, considere eliminarlo del código base.
Actualizar: Fijado en PR # 10 y comprometerse 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L02] El administrador de SafeGuard puede asignar el rol de creador a cualquier dirección
El SafeGuard
contrato define el papel de un CREATOR_ROLE
que, como su nombre indica, se asigna al creador de la salvaguardia.
Sin embargo, al invocar las grantRole
función de la AccessControlEnumerable
contrato en la biblioteca de contratos de OpenZeppelin, un administrador puede otorgar esta función a cualquier dirección. Esto podría causar confusión porque el creador de la SafeGuard
solo puede ser el SafeGuardFactory
.
A lo largo del código base, este rol se ha utilizado solo para restringir que los usuarios interactúen con el setTimelock
función de las SafeGuard
contrato. Por diseño, el sistema asegura que setTimelock
función solo se puede llamar una vez, de en la pestaña SafeGuardFactory
contrato.
Considere quitar el CREATOR_ROLE
papel de la SafeGuard
contrato y utilizando el onlyOwner
modificador existentes setTimelock
función.
Actualizar: Fijado en PR # 10.
[L03] Definición e implementación de interfaz incorrecta
El ISafeGuard
La interfaz no define el queueTransactionWithDescription
función implementado en el SafeGuard
contrato, y al mismo tiempo, define el __abdicar, __queueSetTimelockPendingAdmin y __executeSetTimelockPendingAdmin funciones pero no están implementadas.
Para mejorar la corrección y la consistencia en el código base, considere refactorizar el ISafeGuard
interfaz para que coincida exactamente con el SafeGuard
puesta en práctica.
Actualizar: Corregido en compromiso 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L04] Cadenas de documentación faltantes
Algunos de los contratos y funciones en el código base carecen de documentación. Por ejemplo, algunas funciones existentes SafeGuard
contrato.
Además, algunas cadenas de documentación utilizan un lenguaje informal, como el que por encima de la setTimelock
en función de la SafeGuard
contrato.
Esto dificulta la comprensión por parte de los revisores de la intención del código, lo cual es fundamental para evaluar correctamente no solo la seguridad sino también la corrección. Además, las cadenas de documentación mejoran la legibilidad y facilitan el mantenimiento. Deben explicar explícitamente el propósito o la intención de las funciones, los escenarios en los que pueden fallar, los roles permitidos para llamarlos, los valores devueltos y los eventos emitidos.
Considere documentar minuciosamente todas las funciones (y sus parámetros) que forman parte de la API pública de los contratos. Las funciones que implementan funciones confidenciales, incluso si no son públicas, también deben documentarse claramente. Al escribir docstrings, considere seguir el Formato de especificación natural de Ethereum (NatSpec).
Actualizar: Parcialmente fijo en PR # 10. Se han agregado cadenas de documentación adecuadas a varias funciones en todo el código base. Sin embargo, además de los cambios actuales, considere realizar los siguientes cambios:
- Añada
description
como el@param
en la cadena de documentación anteriorqueueTransactionWithDescription
función - Añada
@param
existentes cadena de documentación por encima de lacreateSafeGuard
Funcionar enSafeGuardFactory
contrato - Añada
@return
en docstrings por encima de las funciones enSafeGuardFactory
contrato.
[L05] Código inútil o repetido
Hay lugares en el código base donde el código se repite o no se necesita. Algunos ejemplos son:
- Líneas 29-32 de las
Registry
contrato son inútiles, porque el_add
función de laEnumerableSet
contrato ya realiza estas comprobaciones contra los valores ya establecidos. - Líneas 62, 67, 73 y 78 de las
SafeGuard
contrato están todos repitiendo la misma operación exacta. Considere encapsularlo en una función interna para evitar la duplicación de código. - Líneas 62-63 y 67 - 68 of
SafeGuard
se repiten. Considere encapsularlos en una sola función interna. - El uso de
gasleft
para especificar cuánto gas se debe reenviar en la llamada de la funciónexecuteTransaction
es innecesario Esto se debe a que, en ese punto de ejecución, todo el gas restante se utilizará para continuar con la ejecución. Si esto no es para ser explícito, considere eliminar elgas
parámetro de la llamada.
Considere aplicar el arreglo sugerido para producir un código más limpio y mejorar la coherencia y la modularidad sobre la base de código.
Actualizar: Fijado en PR # 10 y comprometerse 7fd27df16fc879d990d36a167a0b6e719e578558
.
Notas e información adicional
[N01] Estilo inconsistente
Hay algunos lugares en la base del código, donde las diferencias de estilo afectan la legibilidad, lo que dificulta la comprensión del código. Algunos ejemplos son:
- El
Registry
contract utiliza diferentes estilos para cadenas de documentos en todo el contrato. - El
SafeGuard
contrato está emitiendo un evento cuandoqueueTransactionWithDescription
se llama pero no se emiten eventos en otras funciones que se ocupan de las transacciones. - En
SafeGuard
contrato, a veces propuesta de se utiliza como parámetro con nombre y, a veces _valor se utiliza.
Teniendo en cuenta el valor que un estilo de codificación coherente agrega a la legibilidad del proyecto, considere aplicar un estilo de codificación estándar con la ayuda de herramientas de linter, como Solhint.
Actualizar: Fijado en PR # 10 y comprometerse 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N02] Licencia faltante
A los siguientes contratos dentro del código base les falta un identificador de licencia SPDX.
Para silenciar las advertencias del compilador y aumentar la coherencia en el código base, considere agregar un identificador de licencia. Mientras lo hace considere referirse a spdx.dev directrices.
Actualizar: Fijado en PR # 10 y comprometerse 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N03] La dependencia de OpenZeppelin Contract no está fijada
Para evitar comportamientos inesperados en caso de que se publiquen cambios importantes en futuras actualizaciones de la Biblioteca de contratos de OpenZeppelin, considere fijar la versión de esta dependencia en el package.json archivo.
Actualizar: Fijado en PR # 10.
[N04] La versión del compilador de Solidity no está anclada
A lo largo de la base de código, considere anclar la versión del Compilador de solidez a su última versión estable. Esto debería ayudar a evitar la introducción de errores inesperados debido a futuras versiones incompatibles. Para elegir una versión específica, los desarrolladores deben considerar tanto las características del compilador que necesita el proyecto como la lista de errores conocidos asociado con cada versión del compilador de Solidity.
Actualizar: Fijado en PR # 10.
[N05] Error tipográfico
En varios casos a lo largo del código base, la palabra role
está mal escrito como rol
. Un ejemplo de ello está en la cadena de documentación dentro del constructor
de las SafeGuard
contrato.
Considere corregir estos errores tipográficos para mejorar la legibilidad del código.
Actualizar: Parcialmente fijo en PR # 10. Mientras que la ortografía de role
se ha corregido, el comentario "establecer el rol de administrador en una dirección de administrador definida" debe ser "establecer el rol de administrador en una dirección de administrador definida". Además, "ejecutar" está mal escrito en el SafeGuard
contrato en línea 69, línea 82, línea 96 y línea 110 y "disponible" está mal escrito en línea 70, línea 83, línea 97, línea 111. Además, considere reemplazar palabras informales como "ir a" in SafeGuard
contrato con alternativas formales como “going to”.
[N06] Declarar uint como uint256
Hay varias ocurrencias en el código base donde las variables se declaran de uint
tipo de datos en lugar de uint256
. Por ejemplo, el eta
variable en el QueueTransactionWithDescription
evento de las SafeGuard
contrato.
Para favorecer la claridad, todas las instancias de uint
debe ser declarado como uint256
.
Actualizar: Fijado en PR # 10 y comprometerse 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N07] Importación no utilizada
El SafeGuard
las importaciones por contrato console
contrato pero nunca lo usa.
Para mejorar la legibilidad del código, considere eliminar las importaciones no utilizadas.
Actualizar: Fijado en PR # 10.
Conclusiones
Se han encontrado una vulnerabilidad alta y varias otras vulnerabilidades menores y se han sugerido recomendaciones y correcciones.
- &
- de la máquina
- Mi Cuenta
- a través de
- la columna Acción
- acciones
- Adicionales
- dirección
- Admin
- Todos
- ya haya utilizado
- Aunque
- abejas
- enfoque
- en torno a
- auditoría
- Básicamente
- "Ser"
- loco
- llamar al
- cases
- Causar
- el cambio
- CHARGE
- código
- Codificación
- Algunos
- Compuesto
- confusión
- consideración
- contiene
- continue
- contrato
- contratos
- podría
- creador
- Current
- datos
- tratar
- Diseño
- desarrolladores
- una experiencia diferente
- descubierto CRISPR
- Elaborar
- ETH
- Evento
- Eventos
- ejemplo
- personal
- Caracteristicas
- Finalmente
- Nombre
- Fijar
- Flexibilidad
- encontrado
- función
- fondos
- futuras
- GAS
- Diezmos y Ofrendas
- objetivo
- gobierno
- Gobernador
- orientaciones
- es
- ayuda
- esta página
- Alta
- Cómo
- HTTPS
- implementado
- aumente
- aumentado
- Interfaz
- IT
- conocido
- idioma
- más reciente
- Biblioteca
- Licencia
- línea
- Lista
- local
- cerrado
- miró
- Realizar
- Match
- modulares
- Multisig
- Otro
- presente
- proyecto
- propuesta
- público
- publicar
- registrarte
- Estrenos
- reporte
- repositorio
- Resultados
- una estrategia SEO para aparecer en las búsquedas de Google.
- EN LINEA
- set
- pólipo
- compartido
- inteligente
- Contratos Inteligentes
- solidez
- Estado
- papa
- te
- A través de esta formación, el personal docente y administrativo de escuelas y universidades estará preparado para manejar los recursos disponibles que derivan de la diversidad cultural de sus estudiantes. Además, un mejor y mayor entendimiento sobre estas diferencias y similitudes culturales permitirá alcanzar los objetivos de inclusión previstos.
- a lo largo de
- equipo
- seguir
- Transacciones
- Actualizaciones
- us
- usuarios
- propuesta de
- Vulnerabilidades
- Carteras
- semana
- QUIENES
- dentro de
- palabras
- la escritura