search

ubiquity / ubiquity-dollar

Ubiquity Dollar (UUSD) smart contracts and user interface.
https://uad.ubq.fi
Apache License 2.0
34 stars 91 forks source link

Security CI #256

Closed 0x4007 closed 1 year ago

0x4007 commented 2 years ago

I was looking at the CI of other projects and realized that we can probably just use the same configs.

  1. Slither CI - Origin
  2. Echidna CI - Uniswap
  3. MythX CI - Uniswap
  4. Snyk CI - Origin

Originally posted by @pavlovcik in https://github.com/ubiquity/ubiquity-dollar/discussions/197#discussioncomment-3606636

sergfeldman commented 2 years ago

@pavlovcik Preliminary offer for the bounty: time < 2 weeks, price 2000 USDC

@0xcodercrane Please, provide your time estimate so I can define a price for bounty hunters.

0x4007 commented 2 years ago

Lol to fork some code should definitely not take two weeks. Probably 0.5 - 1 week imo. Also the developer should be writing the quote on their own behalf if that is the source. I would take their GitHub into consideration for the bid.

0xcodercrane commented 2 years ago

First of all, we need to look into more about 4 ci features. That would be cool if @pavlovcik can write down pros/cons (especially pros) and then which feature should be implemented on our CI. You can say like "Hey Origin Slither CI is pretty cool in terms of integration CI", "If we get the Mythx CI implemented on our workflow, we can give more fast ci/cd to bounty hunters"

I think both following stuff needs to be cleared before determining the price and timeline

If we do research about 2 questions primarily and provide estimated time, it will be more clear to bounty hunters and they can implement the features that we want as soon as possible.

Unfortunately, I didn't get a chance to look at their codebase in more details.

0x4007 commented 2 years ago

I just figured that getting more security coverage is better since none of it should be mutually exclusive of eachother. I posted in order of interest, with slither first, then ending with Snyk.

The bottom two require paid subscriptions so I would be inclined to pay just for a month, do some security checks. Cancel, then pay again just before launch basically.

Security is something we should take seriously, and given that a lot of the scripts are already implemented for other open source crypto projects, I don't imagine we have much more custom work to do in order to adapt their code to our repo.

I'm not sure if that answers your questions @0xcodercrane but as long as we can run these tools before launch then I think we're okay.

0xcodercrane commented 2 years ago

I did a quick research about both slither and snyk via their document and github. Slither is a sort of static solidity analyzer tool which is used to find well-known vulnerabilities in solidity codebase. snyk is also a nice tool to find vulnerabilities for the whole workspace. In terms of timeline, I agree with @pavlovcik 's opinion. @sergfeldman . 3 days ~ 1 week should be pretty enough to get this done. I recommend 3 days.

sunny0714 commented 2 years ago

Hey @pavlovcik Nice comments. I also had a quick research on slither and snyk. Hope to handle this

0x4007 commented 2 years ago

Hey @pavlovcik Nice comments. I also had a quick research on slither and snyk. Hope to handle this

Sure thing you're authorized to proceed!

I assume that you won't be able to fully test until we pay for a MythX and Synk keys, so what I recommend is allowing CI to pass even if they do not run. Hopefully we can obtain a free trial of sorts to verify that your scripts work.

sunny0714 commented 2 years ago

Cool, let me start

sunny0714 commented 2 years ago

Hey @pavlovcik Slither seems to work correctly One PR(https://github.com/ubiquity/ubiquity-dollar/pull/268) was opened on feature/security-ci branch Here goes the build link

Going to keep checking others

0x4007 commented 2 years ago

lol wow thats a ton of issues

mangetomat_agadvgkaaiafavm

0xcodercrane commented 2 years ago

Here is my concern @sunny0714 . It is out of concept to move node_modules from root directory to contracts package directory. Another stuff I want to let you know here is that we're doing foundry conversion. Foundry actually doesn't have node_modules. installed in it. so I guess we have to find a way to do that without moving node_modules and work with foundry as well.

sunny0714 commented 2 years ago

I need your right fresh eyes on this(https://github.com/ubiquity/ubiquity-dollar/pull/268#issuecomment-1254688855) @pavlovcik @0xcodercrane

sunny0714 commented 2 years ago

Snyk is also working on this test(https://github.com/sunny0714/ubiquity-dollar/actions/runs/3109985281/jobs/5040872251)

sunny0714 commented 2 years ago

Echidna test case: https://github.com/sunny0714/ubiquity-dollar/actions/runs/3109985283/jobs/5040722797#step:6:80 Slither test case: https://github.com/sunny0714/ubiquity-dollar/actions/runs/3109985280/jobs/5040722798#step:5:353 Now only generating "Unknown file: @openzeppelin/contracts/access/AccessControl.sol" Seems all is working

0x4007 commented 2 years ago

Unknown file: @openzeppelin/contracts/access/AccessControl.sol

I don't recognize this issue. I believe you introduced it. Can you fix it?

0xcodercrane commented 1 year ago

I think its done

0x4007 commented 1 year ago

I think its done

Build failed on the referenced PR so what's the deal here? https://github.com/ubiquity/ubiquity-dollar/pull/272/checks Please confirm the status of what security checks work (I understand that we disabled the paid ones) but we should still be testing to see if they work by spending some money on credits, before accepting this PR and paying the bounty.

Also if possible, please link the PR that is used to claim the bounty. The linked PR is incorrect as it was not merged. This evening reviewing the bounties, this is the second instance where it is not clear to me what code is getting paid for what bounty.

Moving forward, hopefully my PR template will solve this issue automatically by asking the Bounty Hunter to link the PR themself.