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

slayers: possible panic in the SerializeTo with receiver of type Raw #4094

Closed
jcp19 opened this issue Jul 28, 2021 · 3 comments · Fixed by #4576
Closed

slayers: possible panic in the SerializeTo with receiver of type Raw #4094

jcp19 opened this issue Jul 28, 2021 · 3 comments · Fixed by #4576
Assignees
Labels
bug Something isn't working

Comments

@jcp19
Copy link
Contributor

jcp19 commented Jul 28, 2021

Hi :)
In the following excerpt, it seems to me that there is the implicit assumption that len(s.Raw) is at least MetaLen. Is this always true? if not, then it is possible that a panic occurs in the slicing operation (s.Raw[:MetaLen]).

// XXX(roosd): This modifies the underlying buffer. Consider writing to data
// directly.
if err := s.PathMeta.SerializeTo(s.Raw[:MetaLen]); err != nil {
return err
}

If this is indeed a problem, then the simplest solution seems to be to remove the slicing operation because s.PathMeta.SerializeTo already performs a bounds check before writing to the buffer passed as argument.

@oncilla
Copy link
Contributor

oncilla commented Jul 28, 2021

Hi @jcp19

There is an implicit assumption here. The only place that raw is set, here:

s.Raw = data[:pathLen]

This means that s.Raw is non-nil, it is guaranteed to be of at least MetaLen length.
However, to protect against future changes, I think we should just drop the slicing as you suggested.

Care to create a PR?

@jcp19
Copy link
Contributor Author

jcp19 commented Jul 28, 2021

Yes, I will take care of this issue and the other occurrences of s.Raw[:MetaLen].

@matzf matzf added the bug Something isn't working label Oct 5, 2022
jiceatscion added a commit to jiceatscion/scion that referenced this issue Jul 16, 2024
@jiceatscion jiceatscion self-assigned this Jul 16, 2024
@jiceatscion
Copy link
Contributor

Since we haven't heard from jcp19, attempting to refactor as suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants