bitburner-official / bitburner-src

Bitburner source code.
Other
843 stars 273 forks source link

BUGFIX: Scripts are killed too late when prestiging #1683

Closed catloversg closed 1 month ago

catloversg commented 1 month ago

A player reports a bug with stock, ns.atExit and soft reset. The gist is that when they sell the stock in ns.atExit, the profit is added to the player's money after the soft reset.

MRE:

/** @param {NS} ns */
export async function main(ns) {
  ns.stock.buyStock("JGN", 100);
  ns.atExit(() => ns.stock.sellStock("JGN", 100));
  setTimeout(() => ns.singularity.softReset(), 10)
  while (true) await ns.asleep(100);
}

After the soft reset, the player has "initital money" + profit instead of just "initital money".

The bug happens because prestigeWorkerScripts() is called after Player.prestigeAugmentation() and before initStockMarket. The flow:

The fix is fairly simple: We only need to call prestigeWorkerScripts() before Player.prestigeAugmentation(). However, the problem is more complicated than that. Because prestigeWorkerScripts() will run ns.atExit, it allows the player to run arbitrary code when the game logic is performing the prestige and be in an incomplete transition state. This can create many subtle bugs/weird behaviors/exploits. This is an exploit that I come up with:

MRE:

/** @param {NS} ns */
export async function main(ns) {
  Player.money = 76e12;
  Player.receiveInvite("Illuminati");
  Factions.Illuminati.alreadyInvited = true;
  Factions.Illuminati.playerReputation = 1e12;
  Player.augmentations = [];
  Player.queuedAugmentations = [];

  ns.singularity.joinFaction("Illuminati");
  ns.singularity.purchaseAugmentation("Illuminati", "Synfibril Muscle");
  ns.atExit(() => {
    ns.singularity.purchaseAugmentation("Illuminati", "QLink")
    ns.singularity.installAugmentations();
  });
  ns.singularity.installAugmentations();
}

In this PoC:

The cost of QLink will be increased after we buy Synfibril Muscle, but we can "reset" that cost requirement with this exploit.

To prevent this kind of problem, this PR does these things:

d0sboots commented 1 month ago

I think this is a good change. However, I wonder if there are not still holes such as doing ns.run or ns.spawn inside atExit, to further get around the prestige.

In the end, some level of that might be impossible to patch completely, and that might be OK. (Certainly there are plenty of other exploits that we acknowledge.)

catloversg commented 1 month ago

I'll take a look at them (run, exec, spawn, etc.). If I find any problems, I'll open an issue/PR.