Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix[OA-audit-N09]: Missing check for non-zero address #4336

Merged
merged 1 commit into from
Dec 22, 2022
Merged

Conversation

md0x
Copy link
Contributor

@md0x md0x commented Dec 21, 2022

Signed-off-by: Pablo Maldonado pablo@umaproject.org

Motivation

OZ identifies the following issue:

The _isDisputeAllowed function in the OptimisticAsserter contract may call
isDisputeAllowed on an optional EscalationManagerInterface contract. In
practice, the policy is to not validate disputers when no escalation manager is specified, so the
function will exit early instead of invoking a function on the zero address.
Nevertheless, in the interest of local reasoning, consider adding a zero-address check on the
em address prior to using it to make external function calls. Also consider swapping line 449
and line 450 so that the em address assignment is only made when this variable is required.

Summary

Add non-zero address check and return early

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
@md0x md0x marked this pull request as ready for review December 21, 2022 14:40
Copy link
Member

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@chrismaree chrismaree merged commit 9543282 into master Dec 22, 2022
@chrismaree chrismaree deleted the md0x/oa-n09 branch December 22, 2022 07:51
Reinis-FRP pushed a commit that referenced this pull request Dec 28, 2022
Co-authored-by: Pablo Maldonado <pablo@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Reinis-FRP added a commit that referenced this pull request Jan 10, 2023
* feat: implement dvm v2 unstake monitor

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: implement dvm v2 stake monitor

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: implement dvm v2 proposal monitor

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: implement dvm v2 deletion proposal monitor

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: implement dvm v2 emergency proposal monitor

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: implement dvm v2 rolled votes monitor

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix dvm monitor logger imports

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: update dvm2.0 upgrade script (#4304)

* feat: update dvm2.0 upgrade script

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

* feat: update starting index

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

* refactor: update comments

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

* feat: update starting index and starting id

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

* fix: governor starting id

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

* fix: add previous voting contracts events

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

* fix: add previous voting contracts events docs

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

* refator: improve comment

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
Co-authored-by: Pablo Maldonado <pablo@umaproject.org>
Co-authored-by: chrismaree <christopher.maree@gmail.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: address issue in CI (#4315)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* build(deps): bump decode-uri-component from 0.2.0 to 0.2.2 (#4301)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: chrismaree <christopher.maree@gmail.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* chore: Bump ethers to 5.7.x in common and sdk (#4214)

Co-authored-by: chrismaree <christopher.maree@gmail.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: add new health check runner package (#4311)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* improve: Add MerkleTree improvements made in across-protocol/contracts-v2 (#4318)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: new release (#4322)

* feat: new release

Captures MerkleTree util change to match across-protocol/contracts-v2

* Update CHANGELOG.md

* Update CHANGELOG.md

* lint

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: Add mainnet forking test case (#4306)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-L03]: Address erronous docstrings and comments (#4320)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N04]: Add additional index paramaters (#4323)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N05]: Address argument name mismatches between interface and implementation (#4324)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N06]: Address Missing event parameter (#4325)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N07]: Fix licencing issues accross the OA (#4326)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N08]: Fix address multicaller payability (#4327)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N09]: Missing check for non-zero address (#4336)

Co-authored-by: Pablo Maldonado <pablo@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: use external functions in oa (#4337)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: remove unused import (#4338)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: dont use named return variables (#4341)

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N10]: Fix redundant code (#4335)

* fix: remove override from base em

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: shared getRequestId function in full policy em

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: simplify require in configureEscalationManager

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: simplify em lookup in oa

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: store callback recipients as local variables in oa

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: redundant return in oa

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: remove forge-std lib

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: add back gitmodules

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: add newline in gitmodules

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: newline in gitmodules

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: shorten variable names

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N12]: Typographical errors (#4339)

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
Signed-off-by: chrismaree <christopher.maree@gmail.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Co-authored-by: Pablo Maldonado <pablo@umaproject.org>
Co-authored-by: chrismaree <christopher.maree@gmail.com>
Co-authored-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-L01]: Limit ability for Full Policy Escalation Manager to setArbitrationResolution (#4319)

* fix: Limit ability for Full Policy EscilationManager to setArbitrationResolution

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* fix

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* Update packages/core/test/foundry/optimistic-asserter/escalation-manager/FullPolicyEscalationManager.t.sol

Signed-off-by: chrismaree <christopher.maree@gmail.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Co-authored-by: Reinis Martinsons <77973553+Reinis-FRP@users.noreply.github.com>
Co-authored-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-L02]: Lack of access control (#4334)

* fix[OA-audit-L02]: Lack of access control on potentially sensitive function

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

* test: test onlyOptimisticAsserter modifier

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>

* fix

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* Update packages/core/test/foundry/optimistic-asserter/escalation-manager/DisputeLimitingEscalationManager.t.sol

Co-authored-by: Reinis Martinsons <77973553+Reinis-FRP@users.noreply.github.com>

* Update packages/core/test/foundry/optimistic-asserter/escalation-manager/BaseEscalationManager.t.sol

Co-authored-by: Reinis Martinsons <77973553+Reinis-FRP@users.noreply.github.com>

* Update packages/core/test/foundry/optimistic-asserter/escalation-manager/DisputeLimitingEscalationManager.t.sol

Co-authored-by: Reinis Martinsons <77973553+Reinis-FRP@users.noreply.github.com>

* fix

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* fix: remove Ownable from base em

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: reorder imports

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
Signed-off-by: chrismaree <christopher.maree@gmail.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Co-authored-by: Pablo Maldonado <pablo@umaproject.org>
Co-authored-by: chrismaree <christopher.maree@gmail.com>
Co-authored-by: Reinis Martinsons <77973553+Reinis-FRP@users.noreply.github.com>
Co-authored-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N15]: Use of magic numbers (#4340)

Co-authored-by: Pablo Maldonado <pablo@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-L04]: Add missing or incomplete docstrings (#4342)

* fix: natspec in oa public methods

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: add docstrings to oa interface

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: add natspec to oa callback recipient interface

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: natspec in base em

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: natspec em interface

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: natspec full policy em

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix:

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: add comments to internal oa functions

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix[OA-audit-N03]: Always send oracle fee to the Store (#4343)

* fix: always pay oracle fee

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: lift zero fee check

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: refactor dmv monitoring to use single entry

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: add readme

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: readme

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: monitor governor transfers

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: monitor uma mints

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: rename common file

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* Revert "fix: rename common file"

This reverts commit e674408.

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: rename dvm monitor

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: improve block update and add comments

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: add token units to readme

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: parse threshold parameters from main entry point

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: update block range only in main entry point

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: update log levels

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: typescript error

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: use tsc

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: update readme

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix: monitor bots after dvm refactor

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix:

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix:

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix:

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix:

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* fix:

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Pablo Maldonado <pablo@umaproject.org>
Signed-off-by: chrismaree <christopher.maree@gmail.com>
Co-authored-by: Pablo Maldonado <pablomaldonadoturci@gmail.com>
Co-authored-by: Pablo Maldonado <pablo@umaproject.org>
Co-authored-by: chrismaree <christopher.maree@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants