Skip to content

Commit f3eeabf

Browse files
bg0d-droidgopherbot
authored andcommitted
windows: avoid length overflow in NewNTString
NewNTString initializes an NTString with RtlInitString, which cannot accurately represent sources whose NUL-terminated byte length exceeds MAX_USHORT. Reject strings whose byte representation cannot fit in the uint16 length fields before calling RtlInitString. Add a boundary round-trip test for NTString. Fixes golang/go#80103 Change-Id: Id38882a952330305e6c23d0adc86a8bf7920d30e Reviewed-on: https://go-review.googlesource.com/c/sys/+/793100 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Damien Neil <dneil@google.com>
1 parent 3cb6647 commit f3eeabf

3 files changed

Lines changed: 44 additions & 2 deletions

File tree

windows/syscall_windows.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,11 +1728,15 @@ func (s *NTUnicodeString) String() string {
17281728
// the more common *uint16 string type.
17291729
func NewNTString(s string) (*NTString, error) {
17301730
var nts NTString
1731-
s8, err := BytePtrFromString(s)
1731+
s8, err := ByteSliceFromString(s)
17321732
if err != nil {
17331733
return nil, err
17341734
}
1735-
RtlInitString(&nts, s8)
1735+
// The source string plus its terminating NUL must fit within MAX_USHORT.
1736+
if len(s8) > MAX_USHORT {
1737+
return nil, syscall.EINVAL
1738+
}
1739+
RtlInitString(&nts, &s8[0])
17361740
return &nts, nil
17371741
}
17381742

windows/syscall_windows_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,6 +1516,43 @@ func TestRoundtripNTUnicodeString(t *testing.T) {
15161516
}
15171517
}
15181518

1519+
func TestRoundtripNTString(t *testing.T) {
1520+
// NTString maximum string length must fit in a uint16, less for terminal NUL.
1521+
maxString := strings.Repeat("*", windows.MAX_USHORT-1)
1522+
for _, test := range []struct {
1523+
s string
1524+
wantErr bool
1525+
}{{
1526+
s: "",
1527+
}, {
1528+
s: "hello",
1529+
}, {
1530+
s: maxString,
1531+
}, {
1532+
s: maxString + "*",
1533+
wantErr: true,
1534+
}, {
1535+
s: "a\x00a",
1536+
wantErr: true,
1537+
}} {
1538+
nts, err := windows.NewNTString(test.s)
1539+
if (err != nil) != test.wantErr {
1540+
t.Errorf("NewNTString(%q): %v, wantErr:%v", test.s, err, test.wantErr)
1541+
continue
1542+
}
1543+
if err != nil {
1544+
if !errors.Is(err, syscall.EINVAL) {
1545+
t.Errorf("NewNTString(%q): %v, want %v", test.s, err, syscall.EINVAL)
1546+
}
1547+
continue
1548+
}
1549+
s2 := nts.String()
1550+
if test.s != s2 {
1551+
t.Errorf("round trip of %q = %q, wanted original", test.s, s2)
1552+
}
1553+
}
1554+
}
1555+
15191556
func TestIsProcessorFeaturePresent(t *testing.T) {
15201557
// according to
15211558
// https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-isprocessorfeaturepresent

windows/types_windows.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ const (
169169
FORMAT_MESSAGE_ARGUMENT_ARRAY = 8192
170170
FORMAT_MESSAGE_MAX_WIDTH_MASK = 255
171171

172+
MAX_USHORT = 0xffff
172173
MAX_PATH = 260
173174
MAX_LONG_PATH = 32768
174175

0 commit comments

Comments
 (0)