UniversityDAO / udao

The official UniversityDAO DApp repository.
GNU Lesser General Public License v2.1
4 stars 4 forks source link

Reading financial data off the blockchain vs reading it off IPFS #45

Closed cartercameron1 closed 2 years ago

cartercameron1 commented 2 years ago

We should be careful when displaying financial information on the front-end. what is submitted to IPFS does not have to matches what is on the blockchain. Theoretically, a malicious actor could create an IPFS file, manually pass it into the propose function and have a mismatch between what the function says it is going to do on IPFS, and what it is actually going to do on the blockchain. If our front-end only uses the blockchain to read the cid, and displays everything using IPFS, somebody could claim they are requesting x dollars, but in the transaction data on the blockchain request a different amount. We talked about malicious mismatched info at our last meeting @CptAstro but wanted to formally bring it up, and hear your thoughts on it.

austin-davis1 commented 2 years ago

When you mention a malicious party creating their own IPFS file, are you referring to them manually uploading their own file or doing it through our frontend?

It wouldn't be possible to do it through our frontend because as soon as IPFS uploads it will immediately call the propose() function in our code using the same data, so a mismatch shouldn't be possible there.

And if they did it manually, they would first have to match the contents of the IPFS file EXACTLY as ours, because otherwise the CID would be different. If I understand what you're saying, you then say they could call the propose() function using the identical CID but a different grant amount. If that's the case that would generate two separate Proposal IDs pointing to the same CID with different grant amounts.

Is that an accurate description of your scenario?

cartercameron1 commented 2 years ago

I am referring to the malicious actor manually uploading their own files to IPFS, then manually calling the propose functions completely bypassing the front-end. After manually uploading To IPFS then taking the cid and manually passing it into propose. Later the front-end reads off the blockchain sees the cid and displays the faulty IPFS info.

What did you mean when you said the contents of the IPFS file has match exactly?

austin-davis1 commented 2 years ago

I meant since the IPFS CID is content-hashed, if the malicious actor wanted to mess with an existing proposal already on IPFS he would need to mimic the data exactly as is, otherwise a different CID would be produced and there would be no clash of faulty data. The Proposal ID depends on what the CID is, and because Proposal ID is content hashed as well, I don't think there's any clash on the IPFS side.

The only problem I could foresee is what I mentioned in my last comment, but that's if we have two Proposal IDs pointing to a singular CID, and that definitely is a problem that should be looked at

cartercameron1 commented 2 years ago

The IPFS CID would match exactly because the malicious actor created it himself. Let me know if there are any flaws in this logic. I upload a file to IPFS saying I want x dollars and it returns a cid. I take the cid and put it into the propose function except in the propose function I ask for y dollars. Now there is an active proposal with a cid pointing to an IPFS file. The front-end then reads the block that points to the faulty cid. Now when people view the front-end they see a request for x dollars, but after the vote it executes for y dollars.

austin-davis1 commented 2 years ago

Oh I thought you originally meant someone was manually uploading a file to IPFS as a way to invalidate the grant amount of an existing IPFS file.

Just so we're clear, when you say pass in the grant amount to the propose function, are you referring to passing it in to the values [] argument of propose?

If that is the case, and manually calling propose() (given they have the gov contract address) is possible like you say, then yes that would be a potential problem of disconnect between what's displayed on the frontend and what would actually happen on the backend. If we DO in fact pass in the grant amount as part of the values [] argument, we can easily access that value from the same event we're getting the Proposal ID and CID from and just pull it entirely off chain and remove the IPFS grant amount altogether.

When I first pushed it onto IPFS that was because I figured not all grants would be paid out in crypto (we would liquidate our stable coin for USD), so some entries might not have a corresponding blockchain value. If we're doing 100% crypto transactions tho, there's no reason to have grant amount on IPFS at all really.

cartercameron1 commented 2 years ago

exactly the malicious actor would pass a number into values that does not match with what is on the ipfs file.

yes the values [] argument on propose.

I am unsure what the plan is regarding actually payment. I would assume we would want to do as much of it on chain as we could, but that is a discussion for another time.

I just wanted to make sure we are not over relying on IPFS to display financial information. As long as you are displaying the values argument I think we will be fine.erc20 transfers might also have a similar problem since those are done calldatas if I remember correctly. I might be wrong about the erc20 thing.

oslfmt commented 2 years ago

Just read through this thread very quickly so might've missed some things. But yeah I think @cartercameron1 brings up a good point. General rule of thumb should be rely on the blockchain as universal source of truth, IPFS for anything we do not store on the blockchain.

austin-davis1 commented 2 years ago

I just added functionality to pull grant amount off the blockchain in my newest PR. Closing this issue