* Deserialize gamecode as packed in PackedGameDataTo
Most packets use a normal 32 bit int but not PackedGameDataTo. Instead,
Innersloth decided to waste a byte by using WritePacked. As v2 game
codes are always negative, this is less efficient, but alas.
It was documented in the pseudocode and I didn't spot the difference
when reviewing the implementation, so I made this mistake when
deduplicating this code.
Test plan:
- Start EHR on at least 2 clients
- Use the game mode "Natural Disasters"
- Observe that pixelated blobs are visible for all players, not just the host
* uopt: stop parsing code to string
* Address review comments
On January 21 Innersloth released a document that details a few new
features. One of these are host chat commands: any chat message that
starts with "/cmd" is forwarded to the host and to the host only.
Implement this feature in Impostor too for implementation parity.
For more info, see their GitHub page:
https://github.com/Innersloth-LLC/AmongUsModdingInformation
From their Discord:
Admin has pushed hotfix v17.1.2 for mobile players! minor changes to the ad rewards to the daily cap (subject to future change) and updated the screen to clarify when the daily cap is reset.
The new Unity version used by Among Us 16.0.5 disables the
"InsecureHttpOption", which blocks UnityWebRequests via HTTP. Therefore
all Impostor servers should now switch to HTTPS.
https://docs.unity3d.com/2022.3/Documentation/ScriptReference/InsecureHttpOption.html
Thanks to TianMengLucky for reporting this and NikoCat233 for
identifying the cause
Among Us always adds a constant amount of roles to this dict, so it
doesn't make sense to clear this dictionary every time.
Furthermore, previously it was possible to trigger a
ConcurrentModificationException when running Deserialize in parallel
with Serialize. This should no longer happen, as new keys are no longer added
It is possible to spawn invalid network objects without an attached
PlayerInfo, and Impostor will throw exceptions everywhere if you happen
to spawn one. Officials do allow spawning objects like this, so allow it
in Impostor for compatibility reasons
For some network objects, like PlayerControl, we expect that there is an
PlayerInfo object associated to them. If there isn't, throw an exception
in this AC category
Force(.*)Async will perform the same function as \1Async but without
throwing exceptions. The old API is maintained for backwards
compatibility reasons.
Murder checks are now shared with the main function that handles
CheckMurder, as ProtectPlayerAsync and ExileAsync aren't used in the
code I didn't make a similar change there. Checks were also synced
between both methods
This prevents situations where a player that requested red has a
different color when rejoining because red is the default color of a
PlayerInfo object. This is most noticeable if the host is late with
rejoining, and a lot of players are alreading in WaitingForHost limbo.
When the host finally rejoins, they will not assign Red immediately to
players that requested Red as it is "in use", and instead assign the
next color.
By recording and restoring this is avoided, and players tend to get
their own colors back in most cases.
This is not something that officials do at the moment, but I don't think
this has an impact on mods, as a normal checkcolor/setcolor cycle will
take place as usual.
These are no longer used, so warn people who do want to edit them.
I don't foresee these getting removed soon though, as we do want to kick
players on old versions for having an old game version
To prevent needing to pull in Impostor.Hazel, use an extension method
instead.
Thanks Niko for researching this part
Co-Authored-By: Niko233 <139348239+NikoCat233@users.noreply.github.com>
* Add diagnostic message to test HTTP server
This makes it possible to diagnose if the reverse proxy setup is working
correctly or if the game configuration is wrong
* Print console log as well
Only print it once to prevent console spam
* Update src/Impostor.Server/Http/HelloController.cs
Co-authored-by: js6pak <me@6pak.dev>
---------
Co-authored-by: js6pak <me@6pak.dev>
I've checked Impostor for issues that are similar to #664, and found
this one: if mods add a custom rpc on PlayerPhysics, Impostor will check
ownership of it, which isn't intentional.
Mods may send RPC's on a ShipStatus that aren't broadcasts. Previously
Impostor would forbid that, which doesn't make sense to me. By moving
down the check it'll allow mods to broadcast RPC's on ShipStatus.
Reported-By: xtracube
When a client disconnects without ever having joined a game, it leaves
behind an empty game on the server. Remove these when a player
disconnects.
Co-Authored-By: js6pak <me@6pak.dev>
One issue with the ColorLimits checks is that it didn't account for mods
that add extra colors, this is fixed by checking if the route followed
is plausible instead of mimicking client logic and only accepting a
single answer that it expected. A false negative is possible where the
host decides to wrap back to the front but there are still custom colors
available at the end of the list. This is acceptable because Impostor
doesn't currently know the amount of colors available and this issue
isn't major enough to ask all mod authors to declare their custom color
amount to the server.
As extra colors should now be handled by this code, it makes sense to
change the category of the unknown colors check to ProtocolExtensions.
For these mods an exemption of the ColorLimits category is no longer
necessary.
Note that mods that allow duplicate colors that do not use a custom rpc
to set colors can only be supported by disabling the ColorLimits
category, this was already the case and I don't think it can be unified
with the existing check.
This commit does what official servers do:
- despawn the PlayerInfo if someone disconnects while in the lobby
- despawn all PlayerInfos at the end of a game when the EndGame screen
is shown. At this point a lot of other netobjects are destroyed as well,
with the PlayerInfos generally done first.
Looks like Innersloth added this but hasn't actually used this. Weird..
When sent it makes the new host perform a full rejoin, which gums up
Impostor's spawning logic. Fixing that causes the client to gum itself
up. Sigh.
After some testing it seems like host migration still works fine, so
this investigation wasn't needed. Committing for posterity.
Co-Authored-By: js6pak <me@6pak.dev>
In 2024.6.18 Innersloth changed the storage location of the PlayerName
from PlayerInfo to PlayerOutfit. If we don't set the PlayerName, we'll
send an incomplete PlayerInfo to new players, so new players are unable
to stay connected as they will not receive complete PlayerInfo objects
before a timeout in the client activates.
Because we publish PlayerName in the API, we can't completely remove
the PlayerName field from PlayerInfo. We also use it internally in a few
places, so I don't this is worth removing.
As tasks used to be part of the GameData, 2024.6.18 has moved them to
the PlayerInfo. Because they are now called on the PlayerInfo of the
correct player, the playerId field in the RPC is no longer necessary.
2024.6.18 adds a NetId field *in front* of the existing fields to help link back to the PlayerInfo. We don't actually need this NetId and assume it's called on the correct PlayerControl, so we ignore it.
In 2024.6.18 Innersloth promoted PlayerInfo to its own network object, and gave it to the server. InnerGameData is now a container that holds these players. This commit implements the changes necessary to handle these separate PlayerInfo network objects.
To prevent excessive modifications in the codebase, which is prone to
issues if a callsite is missed, automatically do this using properties.
Keep the access modifiers intact for now.
I'm seeing multiple warnings in my server log about forbidden scene
changes. Unfortunately this warning does not include the actual scene
that was triggering the issue, which makes diagnosis/remediation difficult.
Additionally change this logging call to use names suitable for
structured logging.
The default ListenIp for the base game server has been set to 0.0.0.0 in
config.json for as long as I remember, and a different value is only
sporadically used. I remember a case where someone was fancy with Split
DNS. 0 is better as a default than localhost, so change the code
accordingly to aligh with config.json
Fixes CVE-2024-30105 (High 7.5/10)
https://github.com/advisories/GHSA-hh2w-p6rv-4g7w
Impostor doesn't use the affected
JsonSerializer.DeserializeAsyncEnumerable method as it doesn't accept
JSON formatted payloads, but the NU1903 warning is shown on all builds,
leading to warning fatigue.
Previously Impostor liked to kill exiled players server side to raise an
event. This is quite early, as the host will SetRole them to a ghost
role later.
However as PlayerInfo is server-owned by default in 2024.6.18, it starts
killing players client side. This caused meetings to immediately
conclude if the final impostor was voted out, ruining the cutscene.
So this commit makes Impostor wait for hosts to actually kill the player
using SetRole. Do set the LastDeathReason property though for players
using the API.
This item is only used in a LogTrace call, so it seems excessive to kick
a player on servers just for a log statement
Co-Authored-By: js6pak <me@6pak.dev>
There are various bugs in the 2024.6.18 implementation of host-authoritive
multiplayer such that I can't recommend players to use it. Fixing these
issues on the server side is impossible, and I can't smell remotely if
clients have applied the necessary fixes. The currently known bugs
include at least:
- Unable to join more than 8 players in a room
- Unable for new players to join if host switched from host authoritive
to server authoritive
- Host is unable to kill if it is in a host authoritive game while it
thinks it is in server authoritive mode
If Innersloth fixes this mode in future game patches, this option may be
reconsidered.
Some people were confused by config-full.json, thinking that this file
should be edited too. This is not the case and the values in this file
don't apply as it was only meant as an example, so rename the file to
make that more clear.
Also update this file to add the options from the Modular Anticheat
Currently, Impostor will show an "Client is in an invalid state."
message if a player tries to join a match where the host or the joining
player is using a version unknown to Impostor.
This message confused a *lot* of people, so this PR changes the API such
that if the version is not registered, the version itself is put in a
compatibility group and compared, assuming no other game versions are
compatible with it. This works fine in most cases, as Among Us network
versions generally increase over time.
Failure on 2222.2.2 (which is ancient at this point, so likely no longer
used) and future cases where Innersloth messes up (which is unlikely) is
an acceptable consequence to me.
miniduikboot