If a pointer write is not atomic, if the GC is running
concurrently, it may observe a partially updated pointer, which
may point to unallocated or already dead memory. Most pointer
writes, like the store instructions generated by the compiler,
are already atomic. But we still need to be careful in places
like memmove. In memmove, we don't know which bits are pointers
(or too expensive to query), so we ensure that all aligned
pointer-sized units are written atomically.
Fixes#36367.
Updates #36101.
Change-Id: I1b3ca24c6b1ac8a8aaf9ee470115e9a89ec1b00b
Reviewed-on: https://go-review.googlesource.com/c/go/+/212626
Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit ffbc02761a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/213684
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously, we accidentally wrote the Proxy-Authorization header for
the initial CONNECT request to the shared ProxyConnectHeader map when
it was non-nil.
Updates #36431Fixes#36433
Change-Id: I5cb414f391dddf8c23d85427eb6973f14c949025
Reviewed-on: https://go-review.googlesource.com/c/go/+/213638
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 249c85d3aa)
Reviewed-on: https://go-review.googlesource.com/c/go/+/213677
Systems where PowerRegisterSuspendResumeNotification returns ERROR_
FILE_NOT_FOUND are also systems where nanotime() is on "program time"
rather than "real time". The chain for this is:
powrprof.dll!PowerRegisterSuspendResumeNotification ->
umpdc.dll!PdcPortOpen ->
ntdll.dll!ZwAlpcConnectPort("\\PdcPort") ->
syscall -> ntoskrnl.exe!AlpcpConnectPort
Opening \\.\PdcPort fails with STATUS_OBJECT_NAME_NOT_FOUND when pdc.sys
hasn't been initialized. Pdc.sys also provides the various hooks for
sleep resumption events, which means if it's not loaded, then our "real
time" timer is actually on "program time". Finally STATUS_OBJECT_NAME_
NOT_FOUND is passed through RtlNtStatusToDosError, which returns ERROR_
FILE_NOT_FOUND. Therefore, in the case where the function returns ERROR_
FILE_NOT_FOUND, we don't mind, since the timer we're using will
correspond fine with the lack of sleep resumption notifications. This
applies, for example, to Docker users.
Updates #35447
Updates #35482Fixes#36377
Change-Id: I9e1ce5bbc54b9da55ff7a3918b5da28112647eee
Reviewed-on: https://go-review.googlesource.com/c/go/+/208317
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/213198
Starting in Windows 8, the wait functions don't take into account
suspend time, even though the monotonic counters do. This results in
timer buckets stalling on resume. Therefore, this commit makes it so
that on resume, we return from the wait functions and recalculate the
amount of time left to wait.
This is a cherry pick of CL 191957 and its cleanup, CL 198417.
Updates #31528Fixes#36376
Change-Id: I0db02cc72188cb620954e87a0180e0a3c83f4a56
Reviewed-on: https://go-review.googlesource.com/c/go/+/193607
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/213197
The test for gopkg.in/yaml.v2@v2 assumes that there are
no future upstream releases. That assumption empirically
does not hold. Backporting fixes to this test is annoying,
and other gopkg.in cases are already reasonably covered,
so remove the problematic test.
Updates #28856
Change-Id: I6455baa1816ac69e02d1ad5d03b82a93e1481a17
Reviewed-on: https://go-review.googlesource.com/c/go/+/205437
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit f0390ffc9d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/205439
Reviewed-by: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Not all closed issues in a given minor milestone are included in that
release, only the ones that have been labeled as CherryPickApproved are.
Update the links to the GitHub issue tracker to include a filter on the
CherryPickApproved label, so that the default view shows only the
backports that were included in a given release. This should more useful
to most people than seeing all backports (considered and approved).
Do this only for Go 1.9.1 and newer releases, as that is when we started
using the CherryPickCandidate and CherryPickApproved labels.
Updates #35988Fixes#36002
Change-Id: I51e07c1bc3ab9c4a5744e8f668c5470adf78bffe
Reviewed-on: https://go-review.googlesource.com/c/go/+/210118
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If a compilation has multiple text sections, code in
textOff must compare the offset argument against the range
for each text section to determine which one it is in.
The comparison looks like this:
if uintptr(off) >= sectaddr && uintptr(off) <= sectaddr+sectlen
If the off value being compared is equal to sectaddr+sectlen then it
is not within the range of the text section but after it. The
comparison should be just '<'.
Fixes#35210
Change-Id: I114633fd734563d38f4e842dd884c6c239f73c95
Reviewed-on: https://go-review.googlesource.com/c/go/+/203817
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 0ae9389609)
Reviewed-on: https://go-review.googlesource.com/c/go/+/203818
Run-TryBot: Carlos Amedee <carlos@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Binary files included in testdata directories can cause Apple’s
notarization service to reject us since they don’t abide by their
strict requirements.
To emulate go mod vendor, remove all _test.go and testdata files
from the vendor directory and update the instructions.
Updates #34986Fixes#35747
Change-Id: I5cde905fc78838d2e3b1519dab4aeee13d8d5356
Reviewed-on: https://go-review.googlesource.com/c/go/+/208227
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Starting with macOS 10.15 (Catalina), Apple now requires all software
distributed outside of the App Store to be notarized. Any binaries we
distribute must abide by a strict set of requirements like code-signing
and having a minimum target SDK of 10.9 (amongst others).
Apple’s notarization service will recursively inspect archives looking to
find notarization candidate binaries. If it finds a binary that does not
meet the requirements or is unable to decompress an archive, it will
reject the entire distribution. From cursory testing, it seems that the
service uses content sniffing to determine file types, so changing
the file extension will not work.
There are some binaries and archives included in our distribution that
are being detected by Apple’s service as potential candidates for
notarization or decompression. As these are files used by tests and some
are intentionally invalid, we don’t intend to ever make them compliant.
As a workaround for this, we base64-encode any binaries or archives that
Apple’s notarization service issues a warning for, as these warnings will
become errors in January 2020.
Updates #34986
Updates #35747
Change-Id: I106fbb6227b61eb221755568f047ee11103c1680
Reviewed-on: https://go-review.googlesource.com/c/go/+/208118
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 8bbfc51d9a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/208220
Reviewed-by: Alexander Rakoczy <alex@golang.org>
dsa.Verify might currently use a nil s inverse in a
multiplication if the public key contains a non-prime Q,
causing a panic. Change this to check that the mod
inverse exists before using it.
Fixes CVE-2019-17596
Change-Id: I94d5f3cc38f1b5d52d38dcb1d253c71b7fd1cae7
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/572809
Reviewed-by: Filippo Valsorda <valsorda@google.com>
(cherry picked from commit 9119dfb0511326d4485b248b83d4fde19c95d0f7)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/575232
On Ubuntu 18.04 I am seeing GDB fail to restore the stack pointer
during this test because stack unwinding can't find the PC. This CL
is essentially a partial revert of CL 23940 and fixes the issue on
s390x.
Fixes#33757
Change-Id: Ib4c41162dc85dc882eb6e248330f4082c3fa94c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/169857
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit d91f7e6637)
Reviewed-on: https://go-review.googlesource.com/c/go/+/200039
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
TestGoGetInsecure verifies that 'go get -insecure' can fetch a
particular package. However, the GOPROXY protocol does not provide a
means for proxies to indicate packages as insecure; thus, proxies
cannot safely serve those packages.
This also squashes the typo fix from CL 167086.
Updates #30571Fixes#33758
Change-Id: I447776dff98bd8ee6eb5055b897b9c7d293e3423
Reviewed-on: https://go-review.googlesource.com/c/go/+/165745
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/199820
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
If a request for a PTR record returned a response with a non-PTR
answer, goLookupPTR would loop forever. Skipping non-PTR answers
guarantees progress through the DNS response.
Fixes#34661
Updates #34660
Change-Id: Ib5e5263243bc34b9e2f85aa2b913c9cd50dbcaa5
Reviewed-on: https://go-review.googlesource.com/c/go/+/198497
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
RFC 7230 is clear about headers with a space before the colon, like
X-Answer : 42
being invalid, but we've been accepting and normalizing them for compatibility
purposes since CL 5690059 in 2012.
On the client side, this is harmless and indeed most browsers behave the same
to this day. On the server side, this becomes a security issue when the
behavior doesn't match that of a reverse proxy sitting in front of the server.
For example, if a WAF accepts them without normalizing them, it might be
possible to bypass its filters, because the Go server would interpret the
header differently. Worse, if the reverse proxy coalesces requests onto a
single HTTP/1.1 connection to a Go server, the understanding of the request
boundaries can get out of sync between them, allowing an attacker to tack an
arbitrary method and path onto a request by other clients, including
authentication headers unknown to the attacker.
This was recently presented at multiple security conferences:
https://portswigger.net/blog/http-desync-attacks-request-smuggling-reborn
net/http servers already reject header keys with invalid characters.
Simply stop normalizing extra spaces in net/textproto, let it return them
unchanged like it does for other invalid headers, and let net/http enforce
RFC 7230, which is HTTP specific. This loses us normalization on the client
side, but there's no right answer on the client side anyway, and hiding the
issue sounds worse than letting the application decide.
Fixes CVE-2019-16276
Change-Id: I6d272de827e0870da85d93df770d6a0e161bbcf1
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/549719
Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>
(cherry picked from commit 1280b868e82bf173ea3e988be3092d160ee66082)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/558776
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This commit changes sendmsg, recvmsg to use nsendmsg, nrecvmsg on AIX.
These syscalls support the new msghdr structure (with Control
and Controllen) which is needed for golang.org/x/net.
Also define SockaddrDataLink.
Fixes#33982
Change-Id: I233fbd24f9eb86648e0d4d50c2b56da3626292d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/170537
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
(cherry picked from commit e014184c43)
Reviewed-on: https://go-review.googlesource.com/c/go/+/193608
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When Host is not valid per RFC 3986, the behavior of Hostname and Port
was wildly unpredictable, to the point that Host could have a suffix
that didn't appear in neither Hostname nor Port.
This is a security issue when applications are applying checks to Host
and expecting them to be meaningful for the contents of Hostname.
To reduce disruption, this change only aims to guarantee the following
two security-relevant invariants.
* Host is either Hostname or [Hostname] with Port empty, or
Hostname:Port or [Hostname]:Port.
* Port is only decimals.
The second invariant is the one that's most likely to cause disruption,
but I believe it's important, as it's conceivable an application might
do a suffix check on Host and expect it to be meaningful for the
contents of Hostname (if the suffix is not a valid port).
There are three ways to ensure it.
1) Reject invalid ports in Parse. Note that non-numeric ports are
already rejected if and only if the host starts with "[".
2) Consider non-numeric ports as part of Hostname, not Port.
3) Allow non-numeric ports, and hope they only flow down to net/http,
which will reject them (#14353).
This change adopts both 1 and 2. We could do only the latter, but then
these invalid hosts would flow past port checks, like in
http_test.TestTransportRejectsAlphaPort. Non-numeric ports weren't fully
supported anyway, because they were rejected after IPv6 literals, so
this restores consistency. We could do only the former, but at this
point 2) is free and might help with manually constructed Host values
(or if we get something wrong in Parse).
Note that net.SplitHostPort and net.Dial explicitly accept service names
in place of port numbers, but this is an URL package, and RFC 3986,
Section 3.2.3, clearly specifies ports as a number in decimal.
net/http uses a mix of net.SplitHostPort and url.Parse that would
deserve looking into, but in general it seems that it will still accept
service names in Addr fields as they are passed to net.Listen, while
rejecting them in URLs, which feels correct.
This leaves a number of invalid URLs to reject, which however are not
security relevant once the two invariants above hold, so can be done in
Go 1.14: IPv6 literals without brackets (#31024), invalid IPv6 literals,
hostnames with invalid characters, and more.
Tested with 200M executions of go-fuzz and the following Fuzz function.
u, err := url.Parse(string(data))
if err != nil {
return 0
}
h := u.Hostname()
p := u.Port()
switch u.Host {
case h + ":" + p:
return 1
case "[" + h + "]:" + p:
return 1
case h:
fallthrough
case "[" + h + "]":
if p != "" {
panic("unexpected Port()")
}
return 1
}
panic("Host is not a variant of [Hostname]:Port")
Fixes CVE-2019-14809
Updates #29098
Change-Id: I7ef40823dab28f29511329fa2d5a7fb10c3ec895
Reviewed-on: https://go-review.googlesource.com/c/go/+/189258
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 61bb56ad63)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/526408
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Apply the following unpublished golang.org/x/net commit.
commit cdfb69ac37fc6fa907650654115ebebb3aae2087
Author: Filippo Valsorda <filippo@golang.org>
Date: Sun Aug 11 02:12:18 2019 -0400
[release-branch.go1.12] http2: limit number of control frames in server send queue
An attacker could cause servers to queue an unlimited number of PING
ACKs or RST_STREAM frames by soliciting them and not reading them, until
the program runs out of memory.
Limit control frames in the queue to a few thousands (matching the limit
imposed by other vendors) by counting as they enter and exit the scheduler,
so the protection will work with any WriteScheduler.
Once the limit is exceeded, close the connection, as we have no way to
communicate with the peer.
Change-Id: I842968fc6ed3eac654b497ade8cea86f7267886b
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/525552
Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>
(cherry picked from commit 589ad6cc5321fb68a90370348a241a5da0a2cc80)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/526069
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Fixes CVE-2019-9512 and CVE-2019-9514
Updates #33606
Change-Id: I282b3e0fa22422d9ea0d07f4a3935685ce4a7433
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/526071
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
For the case where the addresses of parameter z and x of the function
shlVU overlap and the address of z is greater than x, x (input value)
can be polluted during the calculation when the high words of x are
overlapped with the low words of z (output value).
Updates #31084Fixes#32940
Change-Id: I9bb0266a1d7856b8faa9a9b1975d6f57dece0479
Reviewed-on: https://go-review.googlesource.com/c/go/+/169780
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 503e6ccd74)
Reviewed-on: https://go-review.googlesource.com/c/go/+/185041
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
There's a race here with fork/exec, enable the close-on-exec flag
for the new file descriptor.
Updates #33405Fixes#33424
Change-Id: Ib1e405c3b48b11c867f183fd13eff8b73d95e3b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/188537
Run-TryBot: Baokun Lee <nototon@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 2d6ee6e89a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/188538
Run-TryBot: Ian Lance Taylor <iant@golang.org>
These functions are compiler generated, and as such are only available
in the internal ABI. Doing this avoids generating an alias symbol.
Doing that avoids confusion between unmangled and mangled type symbols.
Updates #30768Fixes#33040
Change-Id: I8aba3934ffa994b1a19fc442cfe3e05642792a25
Reviewed-on: https://go-review.googlesource.com/c/go/+/186278
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Session resumption is not a reliable TLS behavior: the server can decide
to reject a session ticket for a number of reasons, or no reason at all.
This makes this non-hermetic test extremely brittle.
It's currently broken on the builders for both TLS 1.2 and TLS 1.3, and
I could reproduce the issue for TLS 1.3 only. As I was debugging it, it
started passing entirely on my machine.
In practice, it doesn't get us any coverage as resumption is already
tested with the recorded exchange tests, and TestVerifyHostname still
provides a smoke test checking that we can in fact talk TLS.
Updates #32978
Change-Id: I63505e22ff7704f25ad700d46e4ff14850ba5d3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/186239
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry-picked from 20e4540e90)
Reviewed-on: https://go-review.googlesource.com/c/go/+/186277
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
A missing operand to mergePoint caused lower to place values
in the wrong blocks.
Includes test, belt+suspenders to do both ssa check and verify
the output (was is how the bug was originally observed).
The fixed bug here is very likely present in Go versions
1.9-1.12 on amd64 and s390x
Updates #32680.
Fixes#32712.
Change-Id: I63e702c4c40602cb795ef71b1691eb704d38ccc7
Reviewed-on: https://go-review.googlesource.com/c/go/+/183059
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 769fda2d51)
Reviewed-on: https://go-review.googlesource.com/c/go/+/183241
Tweak the previous fix for issue 32673 (in CL 182958) to work around
problems with c-shared build mode that crop up on some of the builders
(10.11, 10.12). We now consistently set vmaddr and vmsize to zero
for the DWARF segment regardless of build mode.
Fixes#32697
Change-Id: Id1fc213590ad00c28352925e2d754d760e022b5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/183237
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/183398
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Cherry Zhang
Bryan C. Mills
Jason A. Donenfeld
Dmitri Shuralyov
Carlos Amedee
Lynn Boger
Andrew
Alexander Rakoczy
Katie Hockman
Michael Munday
Tobias Klauser
Brad Fitzpatrick
Filippo Valsorda
Filippo Valsorda
Clément Chigot
obei
Ian Lance Taylor
erifan01
Baokun Lee
David Chase
Than McIntosh