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

runtime error: slice bounds out of range #941

Open
vladtreny opened this issue May 29, 2022 · 7 comments
Open

runtime error: slice bounds out of range #941

vladtreny opened this issue May 29, 2022 · 7 comments

Comments

@vladtreny
Copy link

Hello,

Sometimes unexpectedly get the following error

panic: runtime error: slice bounds out of range [:1378] with capacity 1315

github.com/google/certificate-transparency-go/asn1.parseField({0x9a98e0?, 0xc004b1b800?, 0xb?}, {0xc0119ec100, 0x562, 0x523}, 0x0, {0x0, 0x0, 0x0, ...})
#011/root/go/pkg/mod/github.com/google/[email protected]/asn1/asn1.go:940 +0x31de
github.com/google/certificate-transparency-go/asn1.UnmarshalWithParams({0xc0119ec100, 0x562, 0x523}, {0x937a20?, 0xc004b1b800?}, {0x0, 0x0})
#011/root/go/pkg/mod/github.com/google/[email protected]/asn1/asn1.go:1190 +0x1b7
github.com/google/certificate-transparency-go/asn1.Unmarshal(...)
#011/root/go/pkg/mod/github.com/google/[email protected]/asn1/asn1.go:1183
github.com/google/certificate-transparency-go/x509.ParseCertificate({0xc0119ec100, 0x562, 0x523})
#011/root/go/pkg/mod/github.com/google/[email protected]/x509/x509.go:2116 +0x95
@pav-kv
Copy link
Contributor

pav-kv commented Jun 10, 2022

@vladtreny Thanks for the report. Could you clarify the conditions under which you get this error? Are you running the CTFE server, or using x509 package standalone?

@vladtreny
Copy link
Author

vladtreny commented Jun 11, 2022

Hello,
I get it from x509 package standalone on high concurrent usage, like 20k goroutines at the same time processing ssls. If see a possibility to reproduce it, will share

@mhutchinson
Copy link
Contributor

I'm not familiar with this code, but I thought I'd take a look for anything obviously unsafe here. The very odd thing is that there's literally a check before the slice function that checks that the parameters are safe. Unless there's something wrong with invalidLength that I'm not seeing, the only other explanation I can see is that the array backing bytes somehow gets smaller. My guess here is that there are concurrent modifications to the input byte slice into the ParseCertificate function.

@vladtreny is it possible that you have code outside of these calls that modifies the slice passed in?

// invalidLength reports whether offset + length > sliceLength, or if the
// addition would overflow.
func invalidLength(offset, length, sliceLength int) bool {
    return offset+length < offset || offset+length > sliceLength
}

... {
    ...
    if invalidLength(offset, t.length, len(bytes)) {
        err = SyntaxError{"data truncated", params.name}
        return
    }
    innerBytes := bytes[offset : offset+t.length] // BOOM

@vladtreny
Copy link
Author

vladtreny commented Jun 13, 2022

Hello,
I process a big array of ssls (like 7 billion).

I do not get this error on 1000 goroutines. But get it on 10k-20k using the same array. Maybe hitting OS limits somewhere, but it is a powerful sever and also tried huge os limits.

On every goroutine, I just parse cert and then loop the data, do not change slices

				cert, err := x509.ParseCertificate(crt.Certificate)
				if err != nil {
					switch err.(type) {
					case nil, x509.NonFatalErrors:
						// ignore
					default:
						continue
					}
				}

Anyway thank you so much for this library, works much better than included in go.

@mhutchinson
Copy link
Contributor

It may be worthwhile to run this with the -race flag to use the go tooling to detect race conditions: https://go.dev/doc/articles/race_detector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
@mhutchinson @pav-kv @vladtreny and others