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

Add a default validation policy #48

Merged
merged 1 commit into from
Nov 22, 2019
Merged

Conversation

hannahhoward
Copy link
Collaborator

Goals

In untrusted context, limit recursive selectors to a fixed depth to prevent some DOS attacks. (should have been done a while ago)

Implementation

  • When a request is received, examine the passed selector by (perhaps a bit meta) traversing the selector spec with a selector, checking all recursive selectors have a depth <= 100 (current fixed depth limit)
  • Does not include customizing allowed max depth for now (future ticket)
  • Does not rule out other kinds of selectors yet
  • also simplifies request tracking in response manager by saving whole request instead of individual components
  • also removes the BuildSelectorSpec method from IPLD bridge because it's a very thin wrapper on ipld prime that needs no mocking.

add a default validation policy that only accepts selectors up to 100 depth
@hannahhoward hannahhoward changed the base branch from feat/send-extensions to master November 19, 2019 15:31
// this selector is a selector for traversing selectors...
// it traverses the various selector types looking for recursion limit fields
// and matches them
s, err := ssb.ExploreRecursive(selector.RecursionLimitNone(), ssb.ExploreFields(func(efsb builder.ExploreFieldsSpecBuilder) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we'll have to revisit this later to take fan-out into account. 100 is small for a chain, but possibly large for a large fanout.

However, we can handle this later. This validation is strictly better than what we have and is a clean way to throw out known-bad selectors.

@anorth
Copy link
Member

anorth commented Nov 22, 2019

FWIW I think this default is more trouble than it will be worth, and we should do the proper solution now of forcing the constructor/caller to specify a value here. It shows immediately in a downstream PR where there's a new flag for "override default validation" explicitly to work around this.

@hannahhoward hannahhoward merged commit 5084f00 into master Nov 22, 2019
@mvdan mvdan deleted the feat/add-default-validation branch December 15, 2021 14:15
marten-seemann pushed a commit that referenced this pull request Mar 2, 2023
* feat(pubsub): refactor to generic interface

Refactor pubsub to operate mostly generically

* refactor(pubsub): use extracted library version

* fix(lint): fix lint errors
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.

3 participants