Skip to content

Commit

Permalink
shiny/screen: sanitize NewWindowOptions.Title.
Browse files Browse the repository at this point in the history
Its bytes are passed to C libraries, so we sanitize as a precaution.

Change-Id: I6ecdd5388be40c4067815ba0484112bea6c55270
Reviewed-on: https://go-review.googlesource.com/37414
Reviewed-by: Alex Brainman <[email protected]>
  • Loading branch information
nigeltao committed Feb 24, 2017
1 parent 35a3e98 commit 8c92c4d
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 26 deletions.
10 changes: 3 additions & 7 deletions shiny/driver/gldriver/cocoa.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,10 @@ func init() {
func newWindow(opts *screen.NewWindowOptions) (uintptr, error) {
width, height := optsSize(opts)

var title string
if opts != nil {
title = opts.Title
}
titlePtr := C.CString(title)
defer C.free(unsafe.Pointer(titlePtr))
title := C.CString(opts.GetTitle())
defer C.free(unsafe.Pointer(title))

return uintptr(C.doNewWindow(C.int(width), C.int(height), titlePtr)), nil
return uintptr(C.doNewWindow(C.int(width), C.int(height), title)), nil
}

func initWindow(w *windowImpl) {
Expand Down
4 changes: 2 additions & 2 deletions shiny/driver/gldriver/x11.c
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ doCloseWindow(uintptr_t id) {
}

uintptr_t
doNewWindow(int width, int height, char* title) {
doNewWindow(int width, int height, char* title, int title_len) {
XSetWindowAttributes attr;
attr.colormap = x_colormap;
attr.event_mask =
Expand Down Expand Up @@ -260,7 +260,7 @@ doNewWindow(int width, int height, char* title) {
XSetWMProtocols(x_dpy, win, atoms, 2);

XSetStandardProperties(x_dpy, win, "", "App", None, (char **)NULL, 0, &sizehints);
XChangeProperty(x_dpy, win, wm_name, utf8_string, 8, PropModeReplace, title, strlen(title));
XChangeProperty(x_dpy, win, wm_name, utf8_string, 8, PropModeReplace, title, title_len);

return win;
}
Expand Down
10 changes: 3 additions & 7 deletions shiny/driver/gldriver/x11.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,13 @@ func init() {
func newWindow(opts *screen.NewWindowOptions) (uintptr, error) {
width, height := optsSize(opts)

var title string
if opts != nil {
title = opts.Title
}
titlePtr := C.CString(title)
defer C.free(unsafe.Pointer(titlePtr))
title := C.CString(opts.GetTitle())
defer C.free(unsafe.Pointer(title))

retc := make(chan uintptr)
uic <- uiClosure{
f: func() uintptr {
return uintptr(C.doNewWindow(C.int(width), C.int(height), titlePtr))
return uintptr(C.doNewWindow(C.int(width), C.int(height), title, C.int(len(title))))
},
retc: retc,
}
Expand Down
8 changes: 2 additions & 6 deletions shiny/driver/internal/win32/win32.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ func newWindow(opts *screen.NewWindowOptions) (syscall.Handle, error) {
if err != nil {
return 0, err
}
var title string
if opts != nil {
title = opts.Title
}
windowTitle, err := syscall.UTF16PtrFromString(title)
title, err := syscall.UTF16PtrFromString(opts.GetTitle())
if err != nil {
return 0, err
}
Expand All @@ -84,7 +80,7 @@ func newWindow(opts *screen.NewWindowOptions) (syscall.Handle, error) {
}
}
hwnd, err := _CreateWindowEx(0,
wcname, windowTitle,
wcname, title,
_WS_OVERLAPPEDWINDOW,
_CW_USEDEFAULT, _CW_USEDEFAULT,
int32(w), int32(h),
Expand Down
5 changes: 1 addition & 4 deletions shiny/driver/x11driver/screen.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,10 +423,7 @@ func (s *screenImpl) NewWindow(opts *screen.NewWindowOptions) (screen.Window, er
)
s.setProperty(xw, s.atomWMProtocols, s.atomWMDeleteWindow, s.atomWMTakeFocus)

var title []byte
if opts != nil {
title = []byte(opts.Title)
}
title := []byte(opts.GetTitle())
xproto.ChangeProperty(s.xc, xproto.PropModeReplace, xw, xproto.AtomWmName, s.atomUTF8String, 8, uint32(len(title)), title)

xproto.CreateGC(s.xc, xg, xproto.Drawable(xw), 0, nil)
Expand Down
28 changes: 28 additions & 0 deletions shiny/screen/screen.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import (
"image"
"image/color"
"image/draw"
"unicode/utf8"

"golang.org/x/image/math/f64"
)
Expand Down Expand Up @@ -238,6 +239,33 @@ type NewWindowOptions struct {
// TODO: fullscreen, icon, cursorHidden?
}

// GetTitle returns a sanitized form of o.Title. In particular, its length will
// not exceed 4096, and it may be further truncated so that it is valid UTF-8
// and will not contain the NUL byte.
//
// o may be nil, in which case "" is returned.
func (o *NewWindowOptions) GetTitle() string {
if o == nil {
return ""
}
return sanitizeUTF8(o.Title, 4096)
}

func sanitizeUTF8(s string, n int) string {
if n < len(s) {
s = s[:n]
}
i := 0
for i < len(s) {
r, n := utf8.DecodeRuneInString(s[i:])
if r == 0 || (r == utf8.RuneError && n == 1) {
break
}
i += n
}
return s[:i]
}

// Uploader is something you can upload a Buffer to.
type Uploader interface {
// Upload uploads the sub-Buffer defined by src and sr to the destination
Expand Down
53 changes: 53 additions & 0 deletions shiny/screen/screen_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright 2017 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package screen

import (
"testing"
)

func TestSanitizeUTF8(t *testing.T) {
const n = 8

testCases := []struct {
s, want string
}{
{"", ""},
{"a", "a"},
{"a\x00", "a"},
{"a\x80", "a"},
{"\x00a", ""},
{"\x80a", ""},
{"abc", "abc"},
{"foo b\x00r qux", "foo b"},
{"foo b\x80r qux", "foo b"},
{"foo b\xffr qux", "foo b"},

// "\xc3\xa0" is U+00E0 LATIN SMALL LETTER A WITH GRAVE.
{"\xc3\xa0pqrs", "\u00e0pqrs"},
{"a\xc3\xa0pqrs", "a\u00e0pqrs"},
{"ab\xc3\xa0pqrs", "ab\u00e0pqrs"},
{"abc\xc3\xa0pqrs", "abc\u00e0pqr"},
{"abcd\xc3\xa0pqrs", "abcd\u00e0pq"},
{"abcde\xc3\xa0pqrs", "abcde\u00e0p"},
{"abcdef\xc3\xa0pqrs", "abcdef\u00e0"},
{"abcdefg\xc3\xa0pqrs", "abcdefg"},
{"abcdefgh\xc3\xa0pqrs", "abcdefgh"},
{"abcdefghi\xc3\xa0pqrs", "abcdefgh"},
{"abcdefghij\xc3\xa0pqrs", "abcdefgh"},

// "世" is "\xe4\xb8\x96".
// "界" is "\xe7\x95\x8c".
{"H 世界", "H 世界"},
{"Hi 世界", "Hi 世"},
{"Hello 世界", "Hello "},
}

for _, tc := range testCases {
if got := sanitizeUTF8(tc.s, n); got != tc.want {
t.Errorf("sanitizeUTF8(%q): got %q, want %q", tc.s, got, tc.want)
}
}
}

0 comments on commit 8c92c4d

Please sign in to comment.