Skip to content

Fix issues with Torch God event#3279

Open
lost-werewolf wants to merge 5 commits intoPryaxis:general-develfrom
lost-werewolf:fix-torch-attack
Open

Fix issues with Torch God event#3279
lost-werewolf wants to merge 5 commits intoPryaxis:general-develfrom
lost-werewolf:fix-torch-attack

Conversation

@lost-werewolf
Copy link
Copy Markdown
Contributor

Fixes two issues relating to the Torch God event:

  • Other clients can't see torch attacks (they can on vanilla servers)
  • Bump range check to 104 tiles for torches - Player.TorchAttack checks 100 blocks, but just in case.

Unsure if the range check should be completely removed for torches. The player can move extremely far away and then they won't be able to relight the torches extinguished by the event, as they'll get rejected by the range check.
Due to a bug or oversight, the client doesn't sync their happyFunTorchTime state when it's set to true, only when it's set to false, so there's no way to know for sure if the player is in the event to apply proper range checking (and projectile creation). Feedback appreciated.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 25, 2026

Greptile Summary

This PR addresses two Torch God event regressions in TShock: hostile TorchGod projectiles were being silently dropped by the server (making the event invisible to other clients), and torch tile-rect updates were being rejected because the 32-tile range check was far too tight for the event's 100-tile attack radius. Both fixes are directionally correct and well-motivated.

Key changes:

  • Bouncer.cs: Adds an early-accept path for ProjectileID.TorchGod when !player.unlockedBiomeTorches, bypassing the Main.projHostile block. The proxy condition is a reasonable workaround for the missing happyFunTorchTime sync, but any player who has never completed the torch unlock can exploit this to send hostile TorchGod projectiles freely, even outside the event. A rate-limit or per-player cooldown would reduce abuse risk.
  • SendTileRectHandler.cs: Adds IsRectATorch to detect single-tile torch edits and widens the range to 104 for those cases. The helper's player parameter is accepted but never used. There is also a question of correctness: the guard Main.tile[rect.X, rect.Y].type == TileID.Torches requires the server tile to already be a torch, which may not hold if the event removes tiles rather than only repainting them — worth confirming against vanilla Player.TorchAttack behavior.

Confidence Score: 3/5

  • Functional improvements are present but the hostile-projectile bypass carries a real exploit risk that should be addressed before merging.
  • The two root fixes are correct in intent. However, the !unlockedBiomeTorches proxy opens a griefing vector where any player who hasn't completed the torch unlock can send TorchGod (hostile) projectiles at will. Additionally, the server-side torch type guard in IsRectATorch may silently fail for relighting extinguished torches, potentially leaving the second fix partially ineffective. These are concrete correctness/security concerns that warrant a targeted revision before merge.
  • TShockAPI/Bouncer.cs — hostile projectile bypass needs a secondary guard; TShockAPI/Handlers/SendTileRectHandler.cs — confirm server-tile type assumption and remove unused parameter

Important Files Changed

Filename Overview
TShockAPI/Bouncer.cs Adds an early-exit path for ProjectileID.TorchGod that bypasses the hostile-projectile block; the proxy condition (!unlockedBiomeTorches) can be abused by any player who has not yet completed the torch unlock, even outside the event.
TShockAPI/Handlers/SendTileRectHandler.cs Introduces IsRectATorch helper and expands the tile-rect range to 104 for torch tiles; the helper has an unused player parameter, and the server-side type guard may prevent the range expansion from applying when a torch was fully removed (rather than just repainted) by the event.

Sequence Diagram

sequenceDiagram
    participant C as Client
    participant B as Bouncer.cs
    participant STR as SendTileRectHandler.cs
    participant S as Server/World

    Note over C,S: Torch God Event Flow

    C->>B: NewProjectile (TorchGod)
    B->>B: Check type == TorchGod && !unlockedBiomeTorches
    alt player has NOT unlocked biome torches
        B->>S: Allow projectile (args.Handled = false)
        S-->>C: Projectile visible to all clients ✓
    else player HAS unlocked biome torches
        B->>B: Main.projHostile[type] == true
        B->>C: Reject + RemoveProjectile ✗
    end

    C->>STR: SendTileRect (torch tile)
    STR->>STR: IsRectATorch(player, rect)
    alt rect.Width==1 && rect.Height==1 && both tiles are TileID.Torches
        STR->>STR: range = 104
    else
        STR->>STR: range = 32 (default)
    end
    STR->>STR: IsInRange(realX, realY, range)
    alt within range
        STR->>S: Apply tile change ✓
    else out of range
        STR-->>C: Reject tile rect ✗
    end
Loading

Reviews (1): Last reviewed commit: "add missing torch check" | Re-trigger Greptile

Comment on lines +1387 to +1392
if (type == ProjectileID.TorchGod && !args.Player.TPlayer.unlockedBiomeTorches)
{
TShock.Log.ConsoleDebug(GetString("Bouncer / OnNewProjectile super accepted from (torch god) {0}", args.Player.Name));
args.Handled = false;
return;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Potential exploit: hostile projectile bypass for non-event players

The condition !args.Player.TPlayer.unlockedBiomeTorches allows any player who has not yet unlocked biome torches to send TorchGod projectiles, regardless of whether they are currently in the event. Because this check is placed before the Main.projHostile[type] gate, it effectively lets a malicious client bypass the hostile-projectile filter simply by never completing the torch unlocking mechanic.

A griefer on a server with multiple users who haven't unlocked biome torches could spam TorchGod projectile packets at will. Even if the projectile itself does not deal direct player damage, it can still trigger server-side effects (tile modifications, audio/visual spam to other clients, etc.) that were intentionally gated behind the hostile-projectile check.

Consider adding an additional server-side guard — for example, a cooldown, a per-player projectile count limit, or logging/rate-limiting suspicious creation spikes — to mitigate abuse until a more authoritative event-state sync is available from the client.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially addressed in a95e88c.
It isn't possible to check if the player is in the torch god event, since the client doesn't send PlayerInfo when setting the flag to true, only when setting it to false.

Comment on lines +629 to +631
// Is the rect actually modifying a torch?
return rect[0, 0].Type == TileID.Torches && Main.tile[rect.X, rect.Y].type == TileID.Torches;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Extended range may not apply when relighting extinguished torches

The condition Main.tile[rect.X, rect.Y].type == TileID.Torches requires the current server-side tile to already be a torch. However, the PR description mentions that the player may need to "relight torches extinguished by the event."

If the Torch God event removes or replaces a torch tile (rather than only modifying its frame/paint), the server tile at that position would no longer have type TileID.Torches at the time the client sends the rect to relight it. In that case, IsRectATorch would return false, the range would stay at 32, and the relighting packet would be rejected — exactly the problem this PR aims to fix.

It's worth clarifying whether the vanilla Player.TorchAttack path only repaints/converts existing torch tiles (in which case the check is fine) or whether it can also replace/remove and later recreate them. If the latter, the server-side type guard should be relaxed or replaced with a check solely on rect[0, 0].Type.

Copy link
Copy Markdown
Contributor Author

@lost-werewolf lost-werewolf Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the torch is removed before the client relights it, it's not a problem. Player.RelightTorches doesn't send a TileSquare on a tile that's either NOT a torch or is NOT an unlit torch, so this shouldn't occur. Lit/unlit torches are the same tile, just with different framing.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant