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

docs: Add sealevel-exploits along with descriptions to docs #2041

Closed
wants to merge 28 commits into from

Conversation

nheingit
Copy link
Contributor

@nheingit nheingit commented Jul 7, 2022

In the process of adding the contents of: https://github.com/coral-xyz/sealevel-attacks to the docs.

This PR will add all of the exploits, as well as describe them in detail, instead of providing just the code.

progress

  • - signer authorization
  • account data matching
  • - owner checks
  • - type cosplay
  • - initialization
  • - arbitrary cpi
  • - duplicate mutable accounts
  • - bump seed canonicalization
  • - pda sharing
  • - Closing accounts (kinda...No explanation here, but all code examples displayed for easy comparison)

@vercel
Copy link

vercel bot commented Jul 7, 2022

@nheingit is attempting to deploy a commit to the 200ms Team on Vercel.

A member of the Team first needs to authorize it.

@nheingit nheingit changed the title Add exploits to docs DOCS: [WIP] Add exploits to docs Jul 7, 2022
@nheingit nheingit changed the title DOCS: [WIP] Add exploits to docs docs: [WIP] Add exploits to docs Jul 7, 2022
@nheingit nheingit marked this pull request as ready for review July 12, 2022 23:36
@nheingit nheingit changed the title docs: [WIP] Add exploits to docs docs: Add sealevel-exploits along with descriptions to docs Jul 12, 2022
@vercel
Copy link

vercel bot commented Jul 13, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
anchor-docs ✅ Ready (Inspect) Visit Preview Dec 6, 2022 at 5:15PM (UTC)

@italoacasas
Copy link
Contributor

Amazing contribution.

I wonder if the first section called "Sealevel Attacks" is redundant now that we have independent articles.

cc @armaniferrante wdyt?

@italoacasas
Copy link
Contributor

@nheingit, you can see all the new pages in your dev env?

I can't see some in the test deployment

@nheingit
Copy link
Contributor Author

Weird.

Yeah I can see them on mine:

Screen Shot 2022-07-13 at 2 09 29 PM

@nheingit
Copy link
Contributor Author

@italoacasas any idea why the tests are failing here? I can't tell what Clippy would be having problems with here...

@italoacasas
Copy link
Contributor

No idea, but I doubt it's related to this PR.

@nheingit
Copy link
Contributor Author

Okay cool, then I won't worry about it 👍

Unrelated to this PR, but don't really want to open an issue for it.

Would I be able to open a PR if I wrote a guide on an updated version of the escrow-program? I was going to put it on my blog, but thought it could live here under the proejcts instead.

Didn't know what kind of review process that would take, or if y'all were interested in having more "anchor approved" projects up on the site.

@callensm
Copy link
Member

if you rebase from master the clippy warnings have been fixed and the tests should pass

@italoacasas
Copy link
Contributor

Would I be able to open a PR if I wrote a guide on an updated version of the escrow program? I was going to put it on my blog but thought it could live here under the projects instead.

It sounds like a good idea.

Didn't know what kind of review process that would take, or if y'all were interested in having more "anchor approved" projects up on the site.

Historically the escrow program has been an excellent example for learning to use anchor/Solana. Go for it.

@ashpoolin
Copy link

Great job, Noah! As best as I can tell the examples are correct, so my suggestions are purely fixes for spelling, grammar, and typos. I even learned a few things while reviewing...

nheingit and others added 3 commits August 4, 2022 12:07
Co-authored-by: ashpoolin <103349226+ashpoolin@users.noreply.github.com>
Co-authored-by: ashpoolin <103349226+ashpoolin@users.noreply.github.com>
Co-authored-by: ashpoolin <103349226+ashpoolin@users.noreply.github.com>
@nheingit
Copy link
Contributor Author

nheingit commented Aug 4, 2022

Thanks for the review @ashpoolin !

@italoacasas is there anything else I need to do to get this merged in?

@italoacasas
Copy link
Contributor

italoacasas commented Aug 4, 2022

@nheingit, my plate is a little insane right now, but I will start reviewing the content tonight. Either way, I'm not an expert on the exploits. We may need @armaniferrante help on this one.

@nheingit
Copy link
Contributor Author

nheingit commented Aug 4, 2022

Don't want it to cause any undue stress! Just wanted to bump this since it had been a little bit. Happy to hop on a call or anything to help. You have me on Twitter I think, and my tg handle is the same if you want to kick off any discussion there.

Co-authored-by: Matthew Callens <callensmatt@gmail.com>
@nheingit nheingit requested a review from callensm August 27, 2022 18:26
@nheingit
Copy link
Contributor Author

nheingit commented Sep 3, 2022

ping @callensm

@Henry-E
Copy link
Contributor

Henry-E commented Dec 5, 2022

I wish i could figure out how to preview this on vercel

@Henry-E
Copy link
Contributor

Henry-E commented Dec 6, 2022

Bloody hell, this rebase thing is a mess, sorry about this. it's harder to fix this particular mess since somehow git has snuck in extra commits between commit: cb46474 and commit: 0101d58 .

Sorry to leave this mess here but i have to run right now. When I get back i will revert back to commit 0101d58 and pick out the extra random commits that git added in.

Maybe then the rebasing will work properly.

@Henry-E Henry-E removed the request for review from callensm December 6, 2022 17:08
@Henry-E
Copy link
Contributor

Henry-E commented Dec 6, 2022

Just trying to authorize the vercel deployment so that I can more easily read through and actually check out what this PR is adding in.

@Henry-E
Copy link
Contributor

Henry-E commented Dec 6, 2022

This honestly seems harmless enough to merge. Will probably do so once the tests finish running (even though they're technically unrelated)

@Henry-E
Copy link
Contributor

Henry-E commented Dec 7, 2022

Upon further reflection I think this just highlights that the anchor site needs a better way to curate and link to relevant blog posts. That way we can keep the main site mostly clean and help people by linking out where needed. For example to all of the great blog posts written by nheingit, rather than hosting them on the anchor site itself.

@nheingit
Copy link
Contributor Author

Just now seeing these @Henry-E

Linking out would be fine. But I only put them up on my site after seeing coral wasn't going to merge this.

@Henry-E
Copy link
Contributor

Henry-E commented Dec 14, 2022

ah ok cool, sorry about the lack of communication. Thanks for posting them on your blog anyway!

@Henry-E Henry-E closed this Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants