From 4e1a03222bfc296c099ff428566a73071335e0ae Mon Sep 17 00:00:00 2001 From: JustArchi Date: Sat, 8 Apr 2017 04:33:01 +0200 Subject: [PATCH] Fix potential STM logic exploit Previously we calculated amounts (and therefore, differences) based on cards only, which means that user could earn some extra 'points' in our calculation for giving us many trash cards that we miss, and in exchange for that swap cards from some other game that would normally results in non-neutral difference on that game basis, but neutral+ in overall basis (that we operated on). This would not cause any serious problem, because ASF ensures that trade is fair before calculating actual neutrality, but it could result in accepting a trade that is neutral+ only at first sight, and non-neutral+ when we take a closer look what we're actually swapping. The logic was enhanced to handle differences on per-game basis now, and taking minimum in account, so all swaps on per-game basis have to be neutral+ now --- ArchiSteamFarm.sln.DotSettings | 3 +- ArchiSteamFarm/ArchiWebHandler.cs | 4 +- ArchiSteamFarm/Trading.cs | 81 +++++++++++++++++-------------- 3 files changed, 48 insertions(+), 40 deletions(-) diff --git a/ArchiSteamFarm.sln.DotSettings b/ArchiSteamFarm.sln.DotSettings index a0608cbd4..b7229d668 100644 --- a/ArchiSteamFarm.sln.DotSettings +++ b/ArchiSteamFarm.sln.DotSettings @@ -375,4 +375,5 @@ True True True - True \ No newline at end of file + True + True \ No newline at end of file diff --git a/ArchiSteamFarm/ArchiWebHandler.cs b/ArchiSteamFarm/ArchiWebHandler.cs index 10f8e41c3..33f1810a4 100644 --- a/ArchiSteamFarm/ArchiWebHandler.cs +++ b/ArchiSteamFarm/ArchiWebHandler.cs @@ -477,7 +477,7 @@ namespace ArchiSteamFarm { return result; } - internal async Task> GetMySteamInventory(bool tradable, HashSet wantedTypes) { + internal async Task> GetMySteamInventory(bool tradable, HashSet wantedTypes, HashSet wantedRealAppIDs = null) { if ((wantedTypes == null) || (wantedTypes.Count == 0)) { Bot.ArchiLogger.LogNullError(nameof(wantedTypes)); return null; @@ -576,7 +576,7 @@ namespace ArchiSteamFarm { steamItem.Type = description.Item2; } - if (!wantedTypes.Contains(steamItem.Type)) { + if (!wantedTypes.Contains(steamItem.Type) || (wantedRealAppIDs?.Contains(steamItem.RealAppID) == false)) { continue; } diff --git a/ArchiSteamFarm/Trading.cs b/ArchiSteamFarm/Trading.cs index 91e84269c..94eeeee80 100644 --- a/ArchiSteamFarm/Trading.cs +++ b/ArchiSteamFarm/Trading.cs @@ -245,75 +245,82 @@ namespace ArchiSteamFarm { return new ParseTradeResult(tradeOffer.TradeOfferID, ParseTradeResult.EResult.AcceptedWithItemLose); } + // Get appIDs we're interested in + HashSet appIDs = new HashSet(tradeOffer.ItemsToGive.Select(item => item.RealAppID)); + // Now check if it's worth for us to do the trade await LimitInventoryRequestsAsync().ConfigureAwait(false); - HashSet inventory = await Bot.ArchiWebHandler.GetMySteamInventory(false, new HashSet { Steam.Item.EType.TradingCard }).ConfigureAwait(false); + HashSet inventory = await Bot.ArchiWebHandler.GetMySteamInventory(false, new HashSet { Steam.Item.EType.TradingCard }, appIDs).ConfigureAwait(false); if ((inventory == null) || (inventory.Count == 0)) { // If we can't check our inventory when not using MatchEverything, this is a temporary failure Bot.ArchiLogger.LogGenericWarning(string.Format(Strings.ErrorIsEmpty, nameof(inventory))); return new ParseTradeResult(tradeOffer.TradeOfferID, ParseTradeResult.EResult.RejectedTemporarily); } - // Get appIDs we're interested in - HashSet appIDs = new HashSet(tradeOffer.ItemsToGive.Select(item => item.RealAppID)); - - // Now remove from our inventory all items we're NOT interested in - inventory.RemoveWhere(item => !appIDs.Contains(item.RealAppID)); - - // If for some reason Valve is talking crap and we can't find mentioned items, this is a temporary failure - if (inventory.Count == 0) { - Bot.ArchiLogger.LogGenericWarning(string.Format(Strings.ErrorIsEmpty, nameof(inventory))); - return new ParseTradeResult(tradeOffer.TradeOfferID, ParseTradeResult.EResult.RejectedTemporarily); - } - // Now let's create a map which maps items to their amount in our EQ - Dictionary amountMap = new Dictionary(); + // This has to be done as we might have multiple items of given ClassID with multiple amounts + Dictionary itemAmounts = new Dictionary(); foreach (Steam.Item item in inventory) { - if (amountMap.TryGetValue(item.ClassID, out uint amount)) { - amountMap[item.ClassID] = amount + item.Amount; + if (itemAmounts.TryGetValue(item.ClassID, out uint amount)) { + itemAmounts[item.ClassID] = amount + item.Amount; } else { - amountMap[item.ClassID] = item.Amount; + itemAmounts[item.ClassID] = item.Amount; } } - // Calculate our value of items to give - List amountsToGive = new List(tradeOffer.ItemsToGive.Count); - Dictionary amountMapToGive = new Dictionary(amountMap); - foreach (ulong key in tradeOffer.ItemsToGive.Select(item => item.ClassID)) { - if (!amountMapToGive.TryGetValue(key, out uint amount)) { + // Calculate our value of items to give on per-game basis + Dictionary> itemAmountToGivePerGame = new Dictionary>(appIDs.Count); + Dictionary itemAmountsToGive = new Dictionary(itemAmounts); + foreach (Steam.Item item in tradeOffer.ItemsToGive) { + if (!itemAmountToGivePerGame.TryGetValue(item.RealAppID, out List amountsToGive)) { + amountsToGive = new List(); + itemAmountToGivePerGame[item.RealAppID] = amountsToGive; + } + + if (!itemAmountsToGive.TryGetValue(item.ClassID, out uint amount)) { amountsToGive.Add(0); continue; } amountsToGive.Add(amount); - amountMapToGive[key] = amount - 1; // We're giving one, so we have one less + itemAmountsToGive[item.ClassID] = amount - 1; // We're giving one, so we have one less } - // Sort it ascending - amountsToGive.Sort(); + // Sort all the lists of amounts to give on per-game basis ascending + foreach (List amountsToGive in itemAmountToGivePerGame.Values) { + amountsToGive.Sort(); + } - // Calculate our value of items to receive - List amountsToReceive = new List(tradeOffer.ItemsToReceive.Count); - Dictionary amountMapToReceive = new Dictionary(amountMap); - foreach (ulong key in tradeOffer.ItemsToReceive.Select(item => item.ClassID)) { - if (!amountMapToReceive.TryGetValue(key, out uint amount)) { + // Calculate our value of items to receive on per-game basis + Dictionary> itemAmountToReceivePerGame = new Dictionary>(appIDs.Count); + Dictionary itemAmountsToReceive = new Dictionary(itemAmounts); + foreach (Steam.Item item in tradeOffer.ItemsToReceive) { + if (!itemAmountToReceivePerGame.TryGetValue(item.RealAppID, out List amountsToReceive)) { + amountsToReceive = new List(); + itemAmountToReceivePerGame[item.RealAppID] = amountsToReceive; + } + + if (!itemAmountsToReceive.TryGetValue(item.ClassID, out uint amount)) { amountsToReceive.Add(0); continue; } amountsToReceive.Add(amount); - amountMapToReceive[key] = amount + 1; // We're getting one, so we have one more + itemAmountsToReceive[item.ClassID] = amount + 1; // We're getting one, so we have one more } - // Sort it ascending - amountsToReceive.Sort(); + // Sort all the lists of amounts to receive on per-game basis ascending + foreach (List amountsToReceive in itemAmountToReceivePerGame.Values) { + amountsToReceive.Sort(); + } - // Check actual difference - // We sum only values at proper indexes of giving, because user might be overpaying - int difference = amountsToGive.Select((t, i) => (int) (t - amountsToReceive[i])).Sum(); + // Calculate final neutrality result + // This is quite complex operation of taking minimum difference from all differences on per-game basis + // When calculating per-game difference, we sum only amounts at proper indexes, because user might be overpaying + int difference = itemAmountToGivePerGame.Min(kv => kv.Value.Select((t, i) => (int) (t - itemAmountToReceivePerGame[kv.Key][i])).Sum()); - // Trade is worth for us if the difference is greater than 0 + // Trade is neutral+ for us if the difference is greater than 0 // If not, we assume that the trade might be good for us in the future, unless we're bot account where we assume that inventory doesn't change return new ParseTradeResult(tradeOffer.TradeOfferID, difference > 0 ? ParseTradeResult.EResult.AcceptedWithItemLose : (Bot.BotConfig.IsBotAccount ? ParseTradeResult.EResult.RejectedPermanently : ParseTradeResult.EResult.RejectedTemporarily)); }