WIZER CTF #23: REQUEST PROCESSOR

Goal

In this challenge, we're uncovering an application logic flaw that has some big security impact.

Description of code

The code below is of a custom Request Processor, which is a NodeJS application that processes custom formatted requests from the user. The expected structure of the request is as stated in the comment: "data:<data>,requestType:<requestType>,key:<key>,". Upon receiving the request, the code splits the request into its parts and validates the key against the secret stored. If the key matches the secret stored, then the action is performed (within an external code, not included in this challenge), otherwise, an "invalid key" error is returned. Furthermore, in the following part of the code, a validation was added to ensure the existence of all the required fields in the request, which returns an error "Invalid request format: data:<data>,requestType:<requestType>,key:<key>," if one of the fields is missing:

What’s wrong with that approach?

An oversight of a minor logic error, makes it possible to trick the system and get in the crack created between the valid and the invalid structures. There are two validations for key existence: (1) const keyFound = keyIndex > 0; which is used to protect the key validation process and (2) A check of `keyIndex < 0` within the part that checks if all the expected elements exist. But what if keyIndex is exactly 0? The logic isn't actually validating the particular expected order of the fields.

What would a successful Code Logic attack look like in this case?

By providing the key as the first data field "key:xyz," in the request, the keyIndex will be 0, which means that we can pass the validation of existence and yet skip the verification of the key against the stored secret, since `keyIndex > 0` isn't true. This way, an attacker could perform any action without the key.

So what?

The showcased vulnerability could cause a variety of risks depending on the functionality of the code which can be overridden. In our little example, the attacker could bypass the key authentication, and practically perform any action without having to have any knowledge of the secret key, however, logic which isn't tight could practically be anywhere and cause almost any type of risks.

Main Takeaways:

• Always make sure your code is air-tight:
  Make sure that the logic of your code is tight and that there are no cracks in the validation process. In this case, the developer could have added a validation to check if the keyIndex is exactly 0, by simply revising `const keyFound = keyIndex > 0;` to `const keyFound = keyIndex >= 0;`.
• Never make any assumptions about the structure of the input:
  Always make sure that the input is provided exactly as expected, and that the validation process is strict. In this case, if the order of the fields is critical to the processing, the developer should have added a validation to check if the fields are in the correct order.
• Always add negative unit tests:
  Make sure to test your code with negative scenarios, to make sure that the code is secure and that the validation process is tight. In this case, a test which checks if the keyIndex is exactly 0, could have found the crack and prevented the vulnerability.