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

Indexes are slurpy and allocates lots of memory #314

Open
Jorropo opened this issue Jul 6, 2022 · 3 comments
Open

Indexes are slurpy and allocates lots of memory #314

Jorropo opened this issue Jul 6, 2022 · 3 comments

Comments

@Jorropo
Copy link
Contributor

Jorropo commented Jul 6, 2022

Indexes datastructure are fully copied into memory when parsing an index.

There is no limit on how big thoses copies were (because practical indexes used by peoples are bigger than what would be a safe limit).
So if you parse an index from untrusted user input, they can send you a really big index and memory hog or OOM you.

Would be nice if we have a way to parse untrusted indexes without opening yourself to OOMs.

This was first attempted in #312 (see discussion in https://github.com/ipld/go-car-priv/pull/2):

But was reverted due to API breaking considerations (see discussion in the channel and https://github.com/ipld/go-car-priv/pull/18)):

See GHSA-9x4h-8wgm-8xfg

go-car@v2.4.0 also includes additional documentation regarding the dangers of consuming CARv2 index data from untrusted sources and a recommendation to regenerate indexes of CAR data from such sources where an index is required.

@masih
Copy link
Member

masih commented Jul 12, 2022

Once streaming index read is implemented, re-add the tests that are removed here

@masih
Copy link
Member

masih commented Jul 14, 2022

The index.ReadFrom tests are moved to run explicitly against that function instead of running as part of inspection. Once the streaming index read is implemented and used as part of inspection, those tests should also run as part of inspection as it was before.

@Jorropo
Copy link
Contributor Author

Jorropo commented Aug 2, 2022

@rvagg we do not use indexes in Kubo, and I have already too much things to do.
I cannot take on that work sorry.

I opened it because as far as I understood in the discussion we had indexes that allocates might be an issue, however boost was not able to do a refactor use io.ReaderAt instead (or / and was concerned of copying into bytes.Buffer due to performance issues I belive).

If consumers of indexes (boost) don't think this is an issue or don't care about it, then we can close this.

@Jorropo Jorropo removed their assignment Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🗄️ Backlog
Development

No branches or pull requests

2 participants