Fix makeslice panic in Dial #5

Author: outdead

.github/workflows/build.yml

@@ -18,7 +18,7 @@ jobs:
     - name: Set up Go 1.x
       uses: actions/setup-go@v2
       with:
-        go-version: ^1.15
+        go-version: ^1.16
 
     - name: Check out code into the Go module directory
       uses: actions/checkout@v2
@@ -27,7 +27,7 @@ jobs:
       uses: golangci/golangci-lint-action@v2
       with:
         # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-        version: v1.33
+        version: v1.42.1
 
         # Optional: working directory, useful for monorepos
         # working-directory: somedir

.golangci.yml

@@ -46,6 +46,8 @@ linters-settings:
   gocyclo:
     # minimal code complexity to report, 30 by default (but we recommend 10-20)
     min-complexity: 15
+  cyclop:
+    max-complexity: 15
   godox:
     keywords:
       - "BUG"
@@ -121,6 +123,7 @@ linters:
   enable-all: true
   disable:
     - gomnd
+    - wrapcheck
 
 issues:
   exclude:

CHANGELOG.md

@@ -3,7 +3,11 @@ All notable changes to this project will be documented in this file.
 
 **ATTN**: This project uses [semantic versioning](http://semver.org/).
 
-## [Unreleased]
+## [Unreleased
+
+## [v1.3.2] - 2022-05-16
+### Fixed
+- Fixed panic: runtime error: makeslice: len out of range in rcon.Dial #5
 
 ## [v1.3.1] - 2021-01-06
 ### Updated
@@ -74,7 +78,8 @@ changed.
 ### Added
 - Initial implementation.
 
-[Unreleased]: https://github.com/gorcon/rcon/compare/v1.3.1...HEAD
+[Unreleased]: https://github.com/gorcon/rcon/compare/v1.3.2...HEAD
+[v1.3.2]: https://github.com/gorcon/rcon/compare/v1.3.1...v1.3.2
 [v1.3.1]: https://github.com/gorcon/rcon/compare/v1.3.0...v1.3.1
 [v1.3.0]: https://github.com/gorcon/rcon/compare/v1.2.4...v1.3.0
 [v1.2.4]: https://github.com/gorcon/rcon/compare/v1.2.3...v1.2.4

rcon.go

@@ -112,7 +112,6 @@ func Dial(address string, password string, options ...Option) (*Conn, error) {
 	if err := client.auth(password); err != nil {
 		// Failed to auth conn with the server.
 		if err2 := client.Close(); err2 != nil {
-			//nolint:errorlint // TODO: Come up with the better wrapping
 			return &client, fmt.Errorf("%w: %v. Previous error: %v", ErrMultiErrorOccurred, err2, err)
 		}
 
@@ -184,22 +183,27 @@ func (c *Conn) auth(password string) error {
 		return err
 	}
 
+	size := response.Size - PacketHeaderSize
+	if size <= 0 {
+		size = response.Size
+	}
+
 	// When the server receives an auth request, it will respond with an empty
 	// SERVERDATA_RESPONSE_VALUE, followed immediately by a SERVERDATA_AUTH_RESPONSE
 	// indicating whether authentication succeeded or failed.
 	// Some servers doesn't send an empty SERVERDATA_RESPONSE_VALUE packet, so we
 	// do this case optional.
 	if response.Type == SERVERDATA_RESPONSE_VALUE {
 		// Discard empty SERVERDATA_RESPONSE_VALUE from authentication response.
-		_, _ = c.conn.Read(make([]byte, response.Size-PacketHeaderSize))
+		_, _ = c.conn.Read(make([]byte, size))
 
 		if response, err = c.readHeader(); err != nil {
 			return err
 		}
 	}
 
 	// We must to read response body.
-	buffer := make([]byte, response.Size-PacketHeaderSize)
+	buffer := make([]byte, size)
 	if _, err := c.conn.Read(buffer); err != nil {
 		return fmt.Errorf("rcon: %w", err)
 	}

rcon_test.go

@@ -17,13 +17,28 @@ import (
 
 func authHandler(c *rcontest.Context) {
 	switch c.Request().Body() {
-	case c.Server().Settings.Password:
-		rcon.NewPacket(rcon.SERVERDATA_RESPONSE_VALUE, c.Request().ID, "").WriteTo(c.Conn())
-		rcon.NewPacket(rcon.SERVERDATA_AUTH_RESPONSE, c.Request().ID, "").WriteTo(c.Conn())
 	case "invalid packet type":
 		rcon.NewPacket(42, c.Request().ID, "").WriteTo(c.Conn())
 	case "another":
 		rcon.NewPacket(rcon.SERVERDATA_AUTH_RESPONSE, 42, "").WriteTo(c.Conn())
+	case "makeslice":
+		size := int32(len([]byte("")) + int(rcon.PacketPaddingSize)) // Some minecraft servers does not add header size
+
+		buffer := bytes.NewBuffer(make([]byte, 0, size+4))
+
+		_ = binary.Write(buffer, binary.LittleEndian, size)
+		_ = binary.Write(buffer, binary.LittleEndian, c.Request().ID)
+		_ = binary.Write(buffer, binary.LittleEndian, rcon.SERVERDATA_RESPONSE_VALUE)
+
+		// Write command body, null terminated ASCII string and an empty ASCIIZ string.
+		buffer.Write(append([]byte(""), 0x00, 0x00))
+
+		buffer.WriteTo(c.Conn())
+
+		rcon.NewPacket(rcon.SERVERDATA_AUTH_RESPONSE, c.Request().ID, "").WriteTo(c.Conn())
+	case c.Server().Settings.Password:
+		rcon.NewPacket(rcon.SERVERDATA_RESPONSE_VALUE, c.Request().ID, "").WriteTo(c.Conn())
+		rcon.NewPacket(rcon.SERVERDATA_AUTH_RESPONSE, c.Request().ID, "").WriteTo(c.Conn())
 	default:
 		rcon.NewPacket(rcon.SERVERDATA_AUTH_RESPONSE, -1, string([]byte{0x00})).WriteTo(c.Conn())
 	}
@@ -129,6 +144,23 @@ func TestDial(t *testing.T) {
 
 		conn.Close()
 	})
+
+	t.Run("makeslice", func(t *testing.T) {
+		server := rcontest.NewServer(
+			rcontest.SetSettings(rcontest.Settings{Password: "makeslice"}),
+			rcontest.SetAuthHandler(authHandler),
+		)
+
+		conn, err := rcon.Dial(server.Addr(), "makeslice")
+		if err != nil {
+			t.Errorf("got err %q, want %v", err, nil)
+			server.Close()
+			return
+		}
+
+		conn.Close()
+		server.Close()
+	})
 }
 
 func TestConn_Execute(t *testing.T) {
@@ -298,42 +330,46 @@ func TestConn_Execute(t *testing.T) {
 		t.Run("pz server", func(t *testing.T) {
 			needle := func() string {
 				n := `List of server commands :
-* addalltowhitelist : Add all the current users connected with a password in the whitelist, so their account is protected.
-* additem : Add an item to a player, if no username is given the item will be added to you, count is optional, use /additem \"username\" \"module.item\" count, ex : /additem \"rj\" \"Base.Axe\" count
-* adduser : Use this command to add a new user in a whitelisted server, use : /adduser \"username\" \"pwd\"
-* addusertowhitelist : Add the user connected with a password in the whitelist, so his account is protected, use : /addusertowhitelist \"username\"
-* addvehicle : Spawn a new vehicle, use: /addvehicle \"script\" \"user or x,y,z\", ex /addvehicle \"Base.VanAmbulance\" \"rj\"
-* addxp : Add experience points to a player, use : /addxp \"playername\" perkname=xp, ex /addxp \"rj\" Woodwork=2
-* alarm : Sound a building alarm at the admin's position.  Must be in a room.
-* banid : Ban a SteamID, use : /banid SteamID
-* banuser : Ban a user, add a -ip to also ban his ip, add a -r \"reason\" to specify a reason for the ban, use : /banuser \"username\" -ip -r \"reason\", ex /banuser \"rj\" -ip -r \"spawn kill\"
-* changeoption : Use this to change a server option, use : /changeoption optionName \"newValue\"
-* chopper : Start the choppers (do noise on a random player)
-* createhorde : Use this to spawn a horde near a player, use : /createhorde count \"username\", ex /createhorde 150 \"rj\", username is optional except from the server console.
-* godmod : Set a player invincible, if no username set it make you invincible, if no value it toggle it, use : /godmode \"username\" -value, ex /godmode \"rj\" -true (could be -false)
-* gunshot : Start a gunshot (do noise on a random player)
+* additem : Give an item to a player. If no username is given then you will receive the item yourself. Count is optional. Use: /additem "username" "module.item" count. Example: /additem "rj" Base.Axe 5
+* adduser : Use this command to add a new user to a whitelisted server. Use: /adduser "username" "password"
+* addvehicle : Spawn a vehicle. Use: /addvehicle "script" "user or x,y,z", ex /addvehicle "Base.VanAmbulance" "rj"
+* addxp : Give XP to a player. Use /addxp "playername" perkname=xp. Example /addxp "rj" Woodwork=2
+* alarm : Sound a building alarm at the Admin's position. (Must be in a room)
+* banid : Ban a SteamID. Use /banid SteamID
+* banuser : Ban a user. Add a -ip to also ban the IP. Add a -r "reason" to specify a reason for the ban. Use: /banuser "username" -ip -r "reason". For example: /banuser "rj" -ip -r "spawn kill"
+* changeoption : Change a server option. Use: /changeoption optionName "newValue"
+* chopper : Place a helicopter event on a random player
+* createhorde : Spawn a horde near a player. Use : /createhorde count "username". Example /createhorde 150 "rj" Username is optional except from the server console. With no username the horde will be created around you
+* createhorde2 : UI_ServerOptionDesc_CreateHorde2
+* godmod : Make a player invincible. If no username is set, then you will become invincible yourself. Use: /godmode "username" -value, ex /godmode "rj" -true (could be -false)
+* gunshot : Place a gunshot sound on a random player
 * help : Help
-* invisible : Set a player invisible zombie will ignore him, if no username set it make you invisible, if no value it toggle it, use : /invisible \"username\" -value, ex /invisible \"rj\" -true (could be -false)
-* kickuser : Kick a user, add a -r \"reason\" to specify a reason for the kick, use : /kickuser \"username\" -r \"reason\"
-* noclip : A player with noclip won't collide on anything, if no value it toggle it, use : /noclip \"username\" -value, ex /noclip \"rj\" -true (could be -false)
-* players : List the players connected
-* quit : Quit the server (but save it before)
-* releasesafehouse : Release a safehouse you are the owner of, use : /releasesafehouse
-* reloadlua : Reload a Lua script, use : /reloadlua \"filename\"
-* reloadoptions : Reload the options on the server (ServerOptions.ini) and send them to the clients
-* removeuserfromwhitelist : Remove the user from the whitelist, use: /removeuserfromwhitelist \"username\"
+* invisible : Make a player invisible to zombies. If no username is set then you will become invisible yourself. Use: /invisible "username" -value, ex /invisible "rj" -true (could be -false)
+* kick : Kick a user. Add a -r "reason" to specify a reason for the kick. Use: /kickuser "username" -r "reason"
+* lightning : Use /lightning "username", username is optional except from the server console
+* noclip : Makes a player pass through walls and structures. Toggles with no value. Use: /noclip "username" -value. Example /noclip "rj" -true (could be -false)
+* players : List all connected players
+* quit : Save and quit the server
+* releasesafehouse : Release a safehouse you own. Use /releasesafehouse
+* reloadlua : Reload a Lua script on the server. Use /reloadlua "filename"
+* reloadoptions : Reload server options (ServerOptions.ini) and send to clients
+* removeuserfromwhitelist : Remove a user from the whitelist. Use: /removeuserfromwhitelist "username"
+* removezombies : UI_ServerOptionDesc_RemoveZombies
+* replay : Record and play replay for moving player. Use /replay "playername" -record|-play|-stop filename. Example: /replay user1 -record stadion.bin
 * save : Save the current world
-* sendpulse : Toggle sending server performance info to this client, use : /sendpulse
-* servermsg : Use this to broadcast a message to all connected players, use : /servermsg my message !
-* setaccesslevel : Use it to set new access level to a player, acces level: admin, moderator, overseer, gm, observer. use : /setaccesslevel \"username\" \"accesslevel\", ex: /setaccesslevel \"rj\" \"moderator\"
-* showoptions : Show the list of current Server options with their values.
-* startrain : Start rain on the server
-* stoprain : Stop rain on the server
-* teleport : Teleport to a player, once teleported, wait 2 seconds to show map, use : /teleport \"playername\" or /teleport \"player1\" \"player2\", ex /teleport \"rj\" or /teleport \"rj\" \"toUser\"
-* teleportto : Teleport to coordinates, use: /teleportto x,y,z, ex /teleportto 100098,189980,0
-* unbanid : Unban a SteamID, use : /unbanid SteamID
-* unbanuser : Unban a player, use : /unbanuser \"username\"
-* voiceban : Block voice from user \"username\", use : /voiceban \"username\" -value, ex /voiceban \"rj\" -true (could be -false)`
+* servermsg : Broadcast a message to all connected players. Use: /servermsg "My Message"
+* setaccesslevel : Set access level of a player. Current levels: Admin, Moderator, Overseer, GM, Observer. Use /setaccesslevel "username" "accesslevel". Example /setaccesslevel "rj" "moderator"
+* showoptions : Show the list of current server options and values.
+* startrain : Starts raining on the server. Use /startrain "intensity", optional intensity is from 1 to 100
+* startstorm : Starts a storm on the server. Use /startstorm "duration", optional duration is in game hours
+* stoprain : Stop raining on the server
+* stopweather : Stop weather on the server
+* teleport : Teleport to a player. Once teleported, wait for the map to appear. Use /teleport "playername" or /teleport "player1" "player2". Example /teleport "rj" or /teleport "rj" "toUser"
+* teleportto : Teleport to coordinates. Use /teleportto x,y,z. Example /teleportto 10000,11000,0
+* thunder : Use /thunder "username", username is optional except from the server console
+* unbanid : Unban a SteamID. Use /unbanid SteamID
+* unbanuser : Unban a player. Use /unbanuser "username"
+* voiceban : Block voice from user "username". Use /voiceban "username" -value. Example /voiceban "rj" -true (could be -false)`
 
 				n = strings.Replace(n, "List of server commands :", "List of server commands : ", -1)