From a9415aff6fcc29949180dc54100a329bcb803789 Mon Sep 17 00:00:00 2001
From: Agniva De Sarker <agnivade@yahoo.co.in>
Date: Mon, 16 Mar 2020 21:52:06 +0530
Subject: [PATCH] MM-22560: Fix crash during slack import (#14054)

* MM-22560: Fix crash during slack import

We were not checking for existence of the users map value
before accessing the Id field. We fix it by checking for
nil pointer first.

Also, we check the length of the Members slice to prevent range
panics.

And finally, while we are here, we move to a switch-case
to make the code a little more idiomatic.

* Adding warn logs when user not found

Co-authored-by: mattermod <mattermod@users.noreply.github.com>
---
 app/slackimport.go      | 33 +++++++++++++++++++++-------
 app/slackimport_test.go | 48 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/app/slackimport.go b/app/slackimport.go
index 35794b9bbb..c4238445b8 100644
--- a/app/slackimport.go
+++ b/app/slackimport.go
@@ -845,30 +845,47 @@ func (a *App) oldImportUser(team *model.Team, user *model.User) *model.User {
 }
 
 func (a *App) oldImportChannel(channel *model.Channel, sChannel SlackChannel, users map[string]*model.User) *model.Channel {
-	if channel.Type == model.CHANNEL_DIRECT {
-		sc, err := a.createDirectChannel(users[sChannel.Members[0]].Id, users[sChannel.Members[1]].Id)
+	switch {
+	case channel.Type == model.CHANNEL_DIRECT:
+		if len(sChannel.Members) < 2 {
+			return nil
+		}
+		u1 := users[sChannel.Members[0]]
+		u2 := users[sChannel.Members[1]]
+		if u1 == nil || u2 == nil {
+			mlog.Warn("Either or both of user ids not found in users.json. Ignoring.", mlog.String("id1", sChannel.Members[0]), mlog.String("id2", sChannel.Members[1]))
+			return nil
+		}
+		sc, err := a.createDirectChannel(u1.Id, u2.Id)
 		if err != nil {
 			return nil
 		}
 
 		return sc
-	}
-
 	// check if direct channel has less than 8 members and if not import as private channel instead
-	if channel.Type == model.CHANNEL_GROUP && len(sChannel.Members) < 8 {
+	case channel.Type == model.CHANNEL_GROUP && len(sChannel.Members) < 8:
 		members := make([]string, len(sChannel.Members))
 
 		for i := range sChannel.Members {
-			members[i] = users[sChannel.Members[i]].Id
+			u := users[sChannel.Members[i]]
+			if u == nil {
+				mlog.Warn("User not found in users.json. Ignoring.", mlog.String("id", sChannel.Members[i]))
+				continue
+			}
+			members[i] = u.Id
 		}
 
-		sc, err := a.createGroupChannel(members, users[sChannel.Creator].Id)
+		creator := users[sChannel.Creator]
+		if creator == nil {
+			return nil
+		}
+		sc, err := a.createGroupChannel(members, creator.Id)
 		if err != nil {
 			return nil
 		}
 
 		return sc
-	} else if channel.Type == model.CHANNEL_GROUP {
+	case channel.Type == model.CHANNEL_GROUP:
 		channel.Type = model.CHANNEL_PRIVATE
 		sc, err := a.CreateChannel(channel, false)
 		if err != nil {
diff --git a/app/slackimport_test.go b/app/slackimport_test.go
index 902f668094..48f2859f6a 100644
--- a/app/slackimport_test.go
+++ b/app/slackimport_test.go
@@ -311,3 +311,51 @@ in this~.`,
 
 	assert.Equal(t, expectedOutput, SlackConvertPostsMarkup(input))
 }
+
+func TestOldImportChannel(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	u1 := th.CreateUser()
+	u2 := th.CreateUser()
+	t.Run("No panic on direct channel", func(t *testing.T) {
+		ch := th.CreateDmChannel(u1)
+		users := map[string]*model.User{
+			th.BasicUser.Id: th.BasicUser,
+		}
+		sCh := SlackChannel{
+			Id:      "someid",
+			Members: []string{u1.Id, "randomID"},
+			Creator: "randomID2",
+		}
+
+		_ = th.App.oldImportChannel(ch, sCh, users)
+	})
+
+	t.Run("No panic on direct channel with 1 member", func(t *testing.T) {
+		ch := th.CreateDmChannel(u1)
+		users := map[string]*model.User{
+			th.BasicUser.Id: th.BasicUser,
+		}
+		sCh := SlackChannel{
+			Id:      "someid",
+			Members: []string{th.BasicUser.Id},
+			Creator: "randomID2",
+		}
+
+		_ = th.App.oldImportChannel(ch, sCh, users)
+	})
+
+	t.Run("No panic on group channel", func(t *testing.T) {
+		ch := th.CreateGroupChannel(u1, u2)
+		users := map[string]*model.User{
+			th.BasicUser.Id: th.BasicUser,
+		}
+		sCh := SlackChannel{
+			Id:      "someid",
+			Members: []string{th.BasicUser.Id},
+			Creator: "randomID2",
+		}
+		_ = th.App.oldImportChannel(ch, sCh, users)
+	})
+}