Review - Trusting the client?

[XavierAntoviaque]

Why do you access a GET variable to get the hack results in
FirstMission, instead of using an object property? This relates to a
part of the codebase that I don't know very well, so I'm unsure. You
never trust anything that directly comes from the client, right?

[XavierAntoviaque (xavier-antoviaque)]

From the discussion:

> [vlad] that GET var is not coming from client input. When the player
> does an action in the game, i use an ajax call to alert the mission
> engine that an action has been taken. The ajax call is a post call
> actually and that's where the var comes from. I get the hack results
> from the game session

Could you show me where this is handled in the code? Do you check what
you get from the client ajax call? Even if it is not coming from player
input in the browser, would a player be able to craft an HTTP request
that would make the mission engine believe the hack was a success when
it was actually a failure?

[Vlad Dragu (vlad-dragu)]

It all starts in the hackit.js file.
On lines 692 and 724 a call the function notify_mission with the param hack_fail and hack_success respectively.
in the function (declaration starts on line 299) i split that param into the action "hack" and the result "success" or "fail" which i sent through the POST ajax call.
Those vars i use in the mission.

I assume that someone could forge the request and send a different hack result. But he will have to know the exact composition of parameters to sent and know exactly how the code looks like in order to be able to exploit it

[XavierAntoviaque (xavier-antoviaque)]

You would be surprised how smart players can be about those things :) Even on the proprietary games on which I've worked, where people don't have access to the code or even the server executables, they still manage to forge this type of requests, just by sniffing the communication between the client and the server.

Here, because it's ajax code, they can simply read the source file and figure it out. The golden rule of "never trusting the client" applies even more in our case.

[Vlad Dragu (vlad-dragu)]

I changed the approach. Right now i rely on the last action registered in the actions table in order to compare it with the active actions queue.
I did the implementation and pushed to the repository but i cannot really test it because the frontend of the missions is not working anymore. I know you did some modifications to it, are you still working on that part of code?