lemon-sh / IRCrarria

A very simple IRC<->Terraria chat bridge for TShock.
GNU General Public License v3.0
6 stars 3 forks source link

Crash when someone sends message in IRC channel #6

Closed ghost closed 1 year ago

ghost commented 1 year ago
nhandled exception
System.UnhandledExceptionEventArgs
[Server API] Error ===================================================================================
[Server API] Error An unhandled exception has occured in TSAPI, and a crash report will be generated
[Server API] Error Generating a crash report, please wait...
[Server API] Error Could not generate a crash report.
[Server API] Error Please upload the crash file and report it at http://tshock.co/
[Server API] Error The process will terminate.
[Server API] Error ===================================================================================
================
1/25/2023 1:31:24 PM: Unhandled Exception
Thread: 12 []
Culture: en-US
Exception: System.ArgumentOutOfRangeException: Length cannot be less than zero. (Parameter 'length')
   at System.String.Substring(Int32 startIndex, Int32 length)
   at IRCrarria.IrcClient.ParsedCommand.Parse(String input)
   at IRCrarria.IrcClient.Start()
   at IRCrarria.IRCrarria.<OnPostInitialize>b__22_0(Object _)
   at System.Threading.Thread.StartHelper.Callback(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.Thread.StartCallback()
HResult: -2146233086
================
Saving world...
Saving world data:

alidating world save:
Backing up world file
World saved.
Unhandled exception. System.ArgumentOutOfRangeException: Length cannot be less than zero. (Parameter 'length')
   at System.String.Substring(Int32 startIndex, Int32 length)
   at IRCrarria.IrcClient.ParsedCommand.Parse(String input)
   at IRCrarria.IrcClient.Start()
   at IRCrarria.IRCrarria.<OnPostInitialize>b__22_0(Object _)
   at System.Threading.Thread.StartHelper.Callback(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.Thread.StartCallback()
[1]    7495 abort      ./TShock.Server
lemon-sh commented 1 year ago

Thanks for submitting an issue. Unfortunately, I cannot reproduce the bug: image image This is TShock 5.1.3.0 on Windows 10 x64.

I've also had this exact config running on this version of TShock on Arch Linux.

Can you provide more details? Like your OS host, the IRC server you're connecting to, the TShock and .NET version you're using, and your ircrarria.toml.

I can't exactly pinpoint where this is happening, I might implement a verbose log toggle in this plugin.

ghost commented 1 year ago
Linux 5.10.0-16-amd64 #1 SMP Debian 5.10.127-1 (2022-06-30) x86_64 GNU/Linux
dotnet version: 6.0.405
tshock: 5.1.3 (TShock-5.1.3-for-Terraria-1.4.4.9-linux-x64-Release.zip)
server: libera.chat

config:

[host]
hostname = "irc.libera.chat"
port = 6697
ssl = true  # change to 'true' if you need TLS
skip_cert_validation = false  # DANGEROUS! Use only when absolutely required.

[irc]
username = "***"
nickname = "***"
channel = "#channel"
prefix = "t!"  # IRC command prefix

# OPTIONAL: specify *raw* IRC commands to run after the bot registers
connect_commands = [
  "PRIVMSG NickServ :IDENTIFY login ***"
]

# OPTIONAL: Additional server info that will be shown when the 'serverinfo' command is used
[server_details]
"Server name" = "server name"
"IP & Port" = "1.2.3.4:7777"
"this server is" = "very cool"
lemon-sh commented 1 year ago

Unfortunately, I couldn't reproduce this on libera.chat either, but I've implemented the aforementioned verbose log functionality.

If you don't want to build the plugin yourself, here's the current master (as of 57953db666102694ae709598e2cdf5946c78655b): IRCrarria-1.4-pre1.zip.

Enable the irc_log option, as shown in the readme, and post the server log where the crash happens (tshock/logs dir), so I can analyze why the parser crashes.

ncfavier commented 1 year ago

It's not hard to figure out what's happening from the provided trace: in the Parse function, the Substring function is called with a negative length parameter. The only place this could be happening is this line

https://github.com/lemon-sh/IRCrarria/blob/f13d8726af079bac35048f93c00c19feb96d2ae5/IrcClient.cs#L103

The length would be negative if trailingIndex < commandIndex, which could happen if the "origin" part of a message contains a colon, which in turn could happen when someone using an IPv6 host address sends a message.

This seems like a simple fix: change

https://github.com/lemon-sh/IRCrarria/blob/f13d8726af079bac35048f93c00c19feb96d2ae5/IrcClient.cs#L93

to

 var trailingIndex = input.IndexOf(':', prefixSpace); 

Or, even better, implement proper parsing of arguments (the trailing colon can only occur as the first character of a word).

ghost commented 1 year ago

Yes, it's because of IPv6

2023-01-28 10:43:56 - IrcClient: INFO: > :user!~user@2a09:bac1:1380:8::49:63 JOIN #channel
2023-01-28 10:43:56 - TShock: ERROR: System.ArgumentOutOfRangeException: Length cannot be less than zero. (Parameter 'length')
   at System.String.Substring(Int32 startIndex, Int32 length)
   at IRCrarria.IrcClient.ParsedCommand.Parse(String input)
   at IRCrarria.IrcClient.Start()
   at IRCrarria.IRCrarria.<OnPostInitialize>b__22_0(Object _)
   at System.Threading.Thread.StartHelper.Callback(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.Thread.StartCallback()
2023-01-28 10:43:56 - Utils: INFO: Broadcast: Saving world...
2023-01-28 10:43:58 - Utils: INFO: Broadcast: World saved.
2023-01-28 10:43:58 - SaveManager: INFO: World saved at
lemon-sh commented 1 year ago

Thanks @ncfavier, you're right! I should've analyzed the parser stack trace more in-depth. I will fix this as soon as I get back home today.

lemon-sh commented 1 year ago

Indeed, I could reproduce the issue on libera.chat when I connected with an IPv6 client. I've released IRCrarria 1.3.1, which contains the fix for this issue.