anoma / namada

Rust implementation of Namada, a Proof-of-Stake L1 for interchain asset-agnostic privacy
https://namada.net
GNU General Public License v3.0
2.39k stars 945 forks source link

All pgf internal address tokens drained with a pgf payment proposal #3532

Closed McDaan closed 1 month ago

McDaan commented 1 month ago

On the current Campfire iteration tududes-test.81efb06d86523ae4f running on the mainnet release candidate v0.40.0, I was digging into PGF type proposals to further improve Undexer/Shielded Live explorer by providing info/feedback in regard to the proposal data. While doing so, I just encountered, which in my opinion is a critical bug or serious at least, because it allowed me to intentionally drain all tokens sitting on the publicgoodfundings internal address to a different address than the steward one.

I have been reading the specs, and I found that semantic is somehow confusing toward this, so I took the tally type literal definition to figure it out. A pgf payment proposal submitted by a steward address has this tally type: LessOneHalfOverOneThirdNay, which means:

LessOneHalfOverOneThirdNay. Requires less than 1/2 of NAY votes over at least 1/3 of the voting power

In this case, there were no votes, no quorum/tally reached (1/3 at least), i.e. no social consensus at all - it basically allows to a malicious actor to potentially stole all tokens on publicgoodfundings internal address.

namadac query-proposal-result --proposal-id 10
Proposal Id: 10
    passed with 0.000000 yay votes, 0.000000 nay votes and 0.000000 abstain votes, total voting power: 690006030.000000, threshold (fraction) of total voting power needed to tally: 230002009.999770 (0.333333333333)

This is the pgf payment proposal submitted by the steward address:

namadac query-proposal --proposal-id 10
Last committed epoch: 1969
Proposal Id: 10
Type: PGF funding
Author: tnam1qr2rdpsxe4uxgxqeu27pdp289sxe8fh7zy3hckmg
Content: {"abstract": "This is a PGF funding proposal.", "authors": "pgf@funding.prop", "created": "2024-07-18T22:00:00Z", "details": "Focusing on RPGF and CPGF.", "discussions-to": "https://x.com/mandragora_spem", "license": "MIT", "motivation": "Proposal to fund public goods with unknown category under (retro)PGF.", "requires": "", "title": "PGF Payment proposal test (Daniel)"}
Start Epoch: 1954
End Epoch: 1956
Activation Epoch: 1958
Status: ended
Data: Actions:
  Continuous: Remove(Internal address=tnam1qz7ws8efr2rmwfyjjjhpgy2xmtfwaceptgl5f63a, amount=0)
  Retroactive: Internal address=tnam1qz7ws8efr2rmwfyjjjhpgy2xmtfwaceptgl5f63a, amount=1856816062272

Balance of address test2 was 0 initally.

Balance of publicgoodfundings internal address few epochs before pgf payment proposal has been submitted:

namadac balance --owner tnam1pgqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqkhgajr --token NAM
nam: 1856816.562272

Balance of address test2 after pgf payment proposal passed (exact amount as requested via rpgf on the proposal):

namadac balance --owner test2 --token NAM
nam: 1856816.062272

Balance of publicgoodfundings internal address few epochs later:

namadac balance --owner tnam1pgqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqkhgajr --token NAM
nam: 12271.161632

Result: the publicgoodfundings internal address has been drained intentionally through a pgf payment proposal that passed without even have reached the required quorum/tally specified on the tally type (1/3 at least).

Fraccaman commented 1 month ago

this is intended behavior. Since stewards are trusted (cause they have been elected by the community), they have the power to create pgf funding proposals which pass by default. If half of 1/3 of the total voting power is voting nay, the funding will fail. 2/3 of the voting power is voting nay, then the funding fails and the steward is removed from being a steward.

McDaan commented 1 month ago

this is intended behavior. Since stewards are trusted (cause they have been elected by the community), they have the power to create pgf funding proposals which pass by default. If half of 1/3 of the total voting power is voting nay, the funding will fail. 2/3 of the voting power is voting nay, then the funding fails and the steward is removed from being a steward.

If no quorum is needed in theory, shouldn't the "total voting power needed to tally" while querying be just 0 if it's intended to pass by default? I mean, if it's an intended behaviour, from a user perspective, I think it could be odd to see props with no "apparent" sufficient social consensus, like prop results query on CLI it's not even displaying the default thing or is having non complete info in regard to this specific tally type design, which focuses on the veto instead of the proposal passing itself.

brentstone commented 1 month ago

If no quorum is needed in theory, shouldn't the "total voting power needed to tally" while querying be just 0 if it's intended to pass by default? I mean, if it's an intended behaviour, from a user perspective, I think it could be odd to see props with no "apparent" sufficient social consensus, like prop results query on CLI it's not even displaying the default thing or is having non complete info in regard to this specific tally type design, which focuses on the veto instead of the proposal passing itself.

Yeah this makes sense, agreed that the log is misleading. This seems to just have been an unintended consequence of how the code is structured, will try to adjust the log / code clarity in this case.

brentstone commented 1 month ago

And just to be clear, tnam1qr2rdpsxe4uxgxqeu27pdp289sxe8fh7zy3hckmg is a steward, right?

brentstone commented 1 month ago

@Fraccaman I think that ca0699f1dd9bc1e10fc57eb01002508e19264cd3 solves this issue to a degree

McDaan commented 1 month ago

And just to be clear, tnam1qr2rdpsxe4uxgxqeu27pdp289sxe8fh7zy3hckmg is a steward, right?

Yes, that's correct.

namadac query-pgf
Pgf stewards:
    - tnam1qr2rdpsxe4uxgxqeu27pdp289sxe8fh7zy3hckmg
      Reward distribution:
      - 1 to tnam1qr2rdpsxe4uxgxqeu27pdp289sxe8fh7zy3hckmg
[...]