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

encoding/asn1: When slicing bytes specify capacity or allocate new slice #14882

Open
rolandshoemaker opened this Issue Mar 19, 2016 · 2 comments

Comments

Projects
None yet
2 participants
@rolandshoemaker
Copy link

rolandshoemaker commented Mar 19, 2016

parseField should use triple slice assignments ([x:y:z]) to specify the capacity of slices when dealing with RawContent and RawValue fields. Defining the capacity of these slices will prevent other fields in a struct that reference the same underlying byte slice from inadvertently being mutated by operations that attempt to expand the slice (i.e. append).

  1. What version of Go are you using (go version)?
    go1.6
  2. What operating system and processor architecture are you using (go env)?
    Ubuntu 15.10 amd64
  3. What did you do?
    Basic example: https://play.golang.org/p/ibPZYcKNs5
package main

import "fmt"
import "encoding/asn1"

type outer struct {
    Raw     asn1.RawContent
    Version int `asn1:"optional,default:1,tag:0"`
    Inner   inner
    Other   asn1.RawValue

}

type inner struct {
    Raw     asn1.RawContent
    Version int `asn1:"optional,default:2,tag:2"`
}

func main() {
    a := outer{Version: 1, Inner: inner{Version:2}, Other: asn1.RawValue{1, 1, false, []byte{10}, []byte{1, 1, 10}}}
    out, _ := asn1.Marshal(a)
    var b outer
    asn1.Unmarshal(out, &b)

    fmt.Println(b.Raw)
    // Since b.Inner.Raw and b.Raw share the shame underlying slice when
    // append extends b.Inner.Raw (since it has no capacity) it is extending
    // into the part of the underlying slice which is referenced by b.Raw and in
    // doing so overwrites a section of that slice
    _ = append((&b).Inner.Raw, []byte{255, 255, 255}...)
    fmt.Println(b.Raw)
}
  1. What did you expect to see?
    In above example b.Raw shouldn't be affected by using append
  2. What did you see instead?
    The last three bytes of b.Raw are overwritten.

@rolandshoemaker rolandshoemaker changed the title encoding/asn1: When slicing bytes specify capacity encoding/asn1: When slicing bytes specify capacity or allocate new slice Mar 20, 2016

@rolandshoemaker

This comment has been minimized.

Copy link
Author

rolandshoemaker commented Mar 20, 2016

Thinking about it setting the capacity may not be the right approach since it means those arrays can never be grown without causing a panic. Perhaps it would be better (although less efficient) to create new slices for each field and copy into them...

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Mar 20, 2016

Or we just document that RawContent aliases other []byte fields.

I'm not sure there's precedent for setting slice capacity in Go APIs as a safety measure.

/cc @agl in case he has an opinion.

@bradfitz bradfitz added this to the Unplanned milestone Mar 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.