From 4c55e4cca794ed4eb3642ff791dd2c3b6ad1a7a1 Mon Sep 17 00:00:00 2001
From: Daniel Canter <dcanter@microsoft.com>
Date: Fri, 26 Feb 2021 02:44:15 -0800
Subject: [PATCH] Fix nil dereference in jobobject.Create and Open

If nil options were passed for `Create` or `Open` both methods would run into a
nil dereference even though for `Create` it states that it will just use default options.

For `Open` you can't open without a name so if nil options are passed just return an error.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
---
 internal/jobobject/jobobject.go      | 27 ++++++++++++++++---------
 internal/jobobject/jobobject_test.go | 30 ++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 9 deletions(-)
 create mode 100644 internal/jobobject/jobobject_test.go

diff --git a/internal/jobobject/jobobject.go b/internal/jobobject/jobobject.go
index 9137c1825a..98e4c51557 100644
--- a/internal/jobobject/jobobject.go
+++ b/internal/jobobject/jobobject.go
@@ -58,6 +58,7 @@ var (
 	ErrNotRegistered = errors.New("job is not registered to receive notifications")
 )
 
+// Options represents the set of configurable options when making or opening a job object.
 type Options struct {
 	// `Name` specifies the name of the job object if a named job object is desired.
 	Name string
@@ -71,17 +72,20 @@ type Options struct {
 
 // Create creates a job object.
 //
-// If name is an empty string, the job will not be assigned a name.
+// If options.Name is an empty string, the job will not be assigned a name.
 //
-// If notifications are not enabled `PollNotifications` will return immediately with error `errNotRegistered`.
+// If options.Notifications are not enabled `PollNotifications` will return immediately with error `errNotRegistered`.
 //
 // If `options` is nil, use default option values.
 //
 // Returns a JobObject structure and an error if there is one.
 func Create(ctx context.Context, options *Options) (_ *JobObject, err error) {
-	var jobName *winapi.UnicodeString
+	if options == nil {
+		options = &Options{}
+	}
 
-	if options != nil && options.Name != "" {
+	var jobName *winapi.UnicodeString
+	if options.Name != "" {
 		jobName, err = winapi.NewUnicodeString(options.Name)
 		if err != nil {
 			return nil, err
@@ -89,7 +93,7 @@ func Create(ctx context.Context, options *Options) (_ *JobObject, err error) {
 	}
 
 	var jobHandle windows.Handle
-	if options != nil && options.UseNTVariant {
+	if options.UseNTVariant {
 		oa := winapi.ObjectAttributes{
 			Length:     unsafe.Sizeof(winapi.ObjectAttributes{}),
 			ObjectName: jobName,
@@ -100,7 +104,11 @@ func Create(ctx context.Context, options *Options) (_ *JobObject, err error) {
 			return nil, winapi.RtlNtStatusToDosError(status)
 		}
 	} else {
-		jobHandle, err = windows.CreateJobObject(nil, jobName.Buffer)
+		var jobNameBuf *uint16
+		if jobName != nil && jobName.Buffer != nil {
+			jobNameBuf = jobName.Buffer
+		}
+		jobHandle, err = windows.CreateJobObject(nil, jobNameBuf)
 		if err != nil {
 			return nil, err
 		}
@@ -118,7 +126,7 @@ func Create(ctx context.Context, options *Options) (_ *JobObject, err error) {
 
 	// If the IOCP we'll be using to receive messages for all jobs hasn't been
 	// created, create it and start polling.
-	if options != nil && options.Notifications {
+	if options.Notifications {
 		mq, err := setupNotifications(ctx, job)
 		if err != nil {
 			return nil, err
@@ -132,13 +140,14 @@ func Create(ctx context.Context, options *Options) (_ *JobObject, err error) {
 // Open opens an existing job object with name provided in `options`. If no name is provided
 // return an error since we need to know what job object to open.
 //
-// If notifications are not enabled `PollNotifications` will return immediately with error `errNotRegistered`.
+// If options.Notifications is false `PollNotifications` will return immediately with error `errNotRegistered`.
 //
 // Returns a JobObject structure and an error if there is one.
 func Open(ctx context.Context, options *Options) (_ *JobObject, err error) {
-	if options != nil && options.Name == "" {
+	if options == nil || (options != nil && options.Name == "") {
 		return nil, errors.New("no job object name specified to open")
 	}
+
 	unicodeJobName, err := winapi.NewUnicodeString(options.Name)
 	if err != nil {
 		return nil, err
diff --git a/internal/jobobject/jobobject_test.go b/internal/jobobject/jobobject_test.go
new file mode 100644
index 0000000000..8169d3d1bd
--- /dev/null
+++ b/internal/jobobject/jobobject_test.go
@@ -0,0 +1,30 @@
+package jobobject
+
+import (
+	"context"
+	"testing"
+)
+
+func TestJobNilOptions(t *testing.T) {
+	_, err := Create(context.Background(), nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestJobCreateAndOpen(t *testing.T) {
+	var (
+		ctx     = context.Background()
+		options = &Options{Name: "test"}
+	)
+
+	_, err := Create(ctx, options)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = Open(ctx, options)
+	if err != nil {
+		t.Fatal(err)
+	}
+}