items parameter for ITrade/SendOffer/v1 should be split into 2 parameters

[dihmeetree]

Let's say I own item with id 2183993 in my OPSkins inventory. My site works in a way that a user can go to the site, load their OPSkins inventory, and then select items they own to deposit onto the site. The site then sends a request to the user via the ITrade/SendOffer/v1 endpoint. Everything is fine and dandy.. the user receives the trade..accepts it and boom done.

Here's the issue however: A user can simply specify an item id that is currently in my sites OPSkins inventory and request to "deposit" that item. Because the ITrade/SendOffer/v1 endpoint doesn't specify which items are who's.. the trade still gets created. However instead of requesting the item from the user.. it thinks i'm trying to give them item to the user.

For reference.. the SteamAPI allows you to specify a toGive and toReceive parameter when u want to send a trade to a user. This allows u to only specify the toReceive parameter in the trade , and will fail if the user didn't actually own the item. Super simple right?

Without validation of the items parameter in the ITrade/SendOffer/v1 request.. I have to do all this extra crap in order to check if the item belongs to the user i'm sending the offer to .. and not my own.

Am I totally overthinking this or am I correct in this assumption? Can someone let me know if there's a way to validate through the api without actually going through the whole inventory of the user?

Thanks in advance :)

[waylaidwanderer]

Aren't you loading the user's inventory already? Just cache the data and use it to validate that the item IDs they selected exist in that inventory. That's what I do on csgoempire.com and it works without any issues.

[dihmeetree]

@waylaidwanderer that's what I'm gonna end up doing.. but I just wanted to see if there was a simpler way -_-

[ha1331]

@waylaidwanderer this feels like a dangerous advice to give. The caching part is what makes this subject to critisism. I would argue this is not a fix, it just makes the window of exploitation smaller. You could make this smarter and invalidate the cache and all that, or you could play with the cache ttl to make it impropable for exploiting and such things, but you only need to do these things because you wanted to use the cache in the first place.

Say you cache for 30 seconds, the user has a bot of their own that accepts trades in a rapid fashion. What they can do is, deposit, accept and proceed to deposit-withdraw the item and if the users inventory is still in the cache, the system you use would see it as them depositing their item but they would just be able to get the item out. Now depending on how you implemented the trades, you might not even catch this when the withdrawal-deposit was accepted. You would just end up rinsed inventory.

I'm expecting this being tried out in the wild very soon.

[waylaidwanderer]

The solution, like you mentioned, is pretty simple. Clear the cache of the user's inventory after the trade is completed. It all comes down to proper implementation.

[ha1331]

@waylaidwanderer well if we're all about proper implementations, why are we even having this conversation? If simple is a metric we should be using, how is this simple compared to:

• just getting the invent from op and not using the cache at all (sure, you get hit with 100ms delay or something compared to using a local, possibly stale cached version) which the user is unlikely to ever notice.

• verifying that the item id's are not known to you before the deposit (you don't have items that have these id's), because if they are, they are not items they can deposit?

Cache invalidation is not a trivial computer science problem. Lots of examples of this going wrong. I'm not saying you can't make this method bulletproof, I'm saying you're having to go thru it because you wanted to use cache in the first place and I'm not sure the added complexity is justified,

I still think the merits of this advice are not self evident, but I don't mind either way. Everybody can go ahead and implement their own way, properly or not.

[ha1331]

I think v2 of ITrade/SendOffer/v1 that follows the steam way of defining theirs and ours is the way to go if you want to make it harder to do this horribly wrong. At least you should mention this looming and not obvious failure case in the docs of ITrade/SendOffer/v1.

[waylaidwanderer]

just getting the invent from op and not using the cache at all (sure, you get hit with 100ms delay or something compared to using a local, possibly stale cached version) which the user is unlikely to ever notice.

This is probably the best way IMO, I only mentioned caching because I thought it was likely that the site would be loading the user's inventory anyway before this step.

[dihmeetree]

Hoping this can be pushed sooner rather than later :/ <3

[mantas7776]

Any updates on this?

[Voyager-Two]

Hello,

It's currently a highest priority ticket & next in-line. I would say it will go out next week, if not, the next.

Thank you for your patience

[waylaidwanderer]

Will the changes be breaking?

[Voyager-Two]

@waylaidwanderer

As with all other changes to our API, no (deliberate) breaking changes. We only make breaking changes if there is a serious bug, otherwise any breaking changes will be made on a new version of the endpoint: v2, v3, etc.

Thanks

[dihmeetree]

Looking forward to this @QuantumLlama <3

[Voyager-Two]

Added items_to_send & items_to_receive on endpoints ITrade/SendOffer & ITrade/SendOfferToSteamId.

Thanks!

[makcc666]

Hello. Methods items_to_send and items_to_receive didnt work Server return 500 error. rawResponse: '500 Internal Server Error', statusCode: 500

Example of request:

{
uid, token, twofactor_code, message: "", items_to_receive: "3122675",
}

[Voyager-Two]

@makcc666

This issue should be resolved now.

Sorry for the inconvenience.

[makcc666]

Thanks for your work!