SmartPy Version Migration Postmortem (Incident #1)
Date: 2022-04-28
Authors: dcale
Status: Complete, action items in progress
Summary: A SmartPy version migration has gone wrong, allowing internal entrypoint to be called externally.
Impact: All internal entrypoints could be called by an external address, effectively allowing someone performing the action in the name of the last user interacting with the contract. No funds were directly at risk at any time during the incident.
Root Causes: SmartPy deprecated @sp.sub_entry_point and @sp.global_lambda in favor of a parametrized @sp.private_lambda. During the migration to the new system our verify_internal lambda was configured to use the parameter wrap_call=False. Since verify_internal
was only verifying whether the caller of the entrypoint was the contract itself and not "eventful" we could see the lambda in the transpiled Michelson but it was never called. Only wrap_call=True will force the lambda even if not "eventful" to be computed at
the defined point.
| Engine | Address | wrap_call=False |
|---|---|---|
| ubtc tez | KT1VjQoL5QvyZtm9m1voQKNTNcQLi5QiGsRZ | OK |
| ubtc tzbtc LB | KT1NFWUqr9xNvVsz2LXCPef1eRcexJz5Q2MH | OK |
| uusd tez | KT1FFE2LC5JpVakVjHm5mM36QVp2p3ZzH4hH | OK |
| uusd tzbtc | KT1VwywA1XM7PL3UVLXA8vmDPvLsGHwgS7wS | NOK |
| uusd tzbtc LB | KT1FzcHaNhmpdYPNTgfb8frYXx7B5pvVyowu | OK |
| udefi uusd | KT1B2GSe47rcMCZTRk294havTpyJ36JbgdeB | OK |
| udefi tez | KT1ALVxK1YPsf1JfyqfivZT3rGCPwvebFZjs | NOK |
| udefi tzbtc LB | KT1U3RkwL3r7wi7tdjFxkjfDGfPrKrYFYGFh | NOK |
Trigger: A manual test routine has shown the unexpected behaviour.
Resolution: The engines have been removed from the list of admin allowing them to mint and burn synthetic assets, as well as the ability to set a YOU weight. New engines with the fix (basically setting wrap_call=True) have been deployed.
Detection: A user in discord complained about not being able to convert.
Action Items:
| Action Item | Type | Owner | State |
|---|---|---|---|
| Identify engines with flaw | mitigate | dcale | DONE |
| Remove the engines from the admin list of the synthetic asset in a multisig ceremony | mitigate | youves signers | DONE |
| Fix the engine code with the finding | prevent | florin | DONE |
| Add internal entrypoint calls unit tests | prevent | florin | DONE |
| Add internal entrypoint calls to deployment testing protocol | prevent | florin | DONE |
| Redeploy the new engines | prevent | florin | DONE |
Lessons Learned#
What went well#
- As soon as the team was made aware of the issue, the mitigation was implemented and deployed within less than 1 hour 30 minutes (including the multisig ceremony).
- The youves multisig parties understood the urgency and reacted on this unplanned issue within 5 minutes. (thank you!!!)
What went wrong#
- Deployment testing procotol did not check the internal entrypoints.
Where we got lucky#
- Unplanned manual test decided to also test the "internal" entrypoints.
set_target_pricechecks the sender.
Conclusion#
- Also when it seems unneccessary the additional check in
set_target_pricemade sure that no unlimited amounts of synthetic assets could be minted. - Manual test protocols are important and need to cover every entrypoint, also if the expected behaviour is a fail.
Timeline#
2022-04-28 (all times UTC)
- 11:40 manual test identifies weird behaviour
- 11:55 engines with behaviour listed in list
- 12:00 preparation of multisig execution payload
- 12:47 multisig ceremony is kicked-off
- 12:51 required signatures are collected
- 13:15 flawed youves engines removed from admin list (ooRPfrFBUdKBkxRoqDvJnu8zUxVywRtTkjQkXBpYbZs1vuwU69i)