Technical analysis of Telco API security challenge - Three must to have API security controls

Tl;dr: Authentication alone could only hide API security weaknesses. Three security controls are required to address the root cause of this API secure programming challenge.

This article is a technical analysis of nearly 40 submissions that we have received for SecDim’s October 2022 Fix the Security Bug secure programming challenge.

The October 2022 challenge was inspired by the largest Australian telco data breach. The challenge is still available. If you wish to try it, you may want to skip reading the rest of this article.

The API

The API was implemented using Open API, Flask and Connextion. It returns the user details for a given user identifier:

curl 'http://localhost:8080/v1.0/users/830350f9-7497-499e-9ba6-ed3eded54802' \
  -H 'accept: application/json'

{
  "dlicense": "234852687",
  "name": "Sam Dastyari",
  "passport": "R22332048",
  "uid": "830350f9-7497-499e-9ba6-ed3eded54802"
}

The API security vulnerabilities

The challenge had the following weakness types:

• CWE-359: Exposure of Private Personal Information to an Unauthorized Actor: This means the API endpoint was left unauthenticated and publicly accessible by anyone.
• CWE-340: Generation of Predictable Numbers or Identifiers: This means sequential user identifiers (e.g. 1, 2, 3, …) are unsafe when we store private user data and we use these identifiers to fetch their details.
• CWE-770: Allocation of Resources Without Limits or Throttling: This means the API endpoint did not enforce rate limiting. With a simple script it was possible to continuously fetch the API.

With exception of CWE-359, for each weakness there was a security integration test to guide the player on what the weakness is and what the expected behavior of the program should be.

There were also a number of usability tests to make sure the API works as intended. A security patch should not cause disruption to the usability of the API (if it doesn’t work, who cares about its security).

The goal of the challenge was to pass all the tests. Meaning fix the security vulnerability while not breaking the API functionality

The vulnerable code

The following code snippet shows the vulnerable part of the program.

def get_user_detail(userId: str) -> Any:
    try:
        _user = search(Users.users, userId)
        if _user:
            return _user, 200
        if Users.users.get(int(userId)):
            return Users.users[int(userId)]
    except Exception as ex:
        return {"message": str(ex)}, 404
    return "Not found", 404


def search(values, searchFor):
    for index in values:
        for key in values[index]:
            if searchFor == values[index][key]:
                return values[index]
    return None

get_user_detail method used an internal method search to look up for a user detail. search was a generic function (copied from StackOverflow). This generic method did not restrict what the lookup field should be and therefore, any field within the user object could be used to get the user detail.

Furthermore, get_user_detail was not rate limited and open to large number of invocations.

The solutions

I have selected three solutions, each highlighting a new API security control.

Solution I

Pros:

• Whitelisted JSON serialized fields

Cons:

• Bad rate limit
• No explicit check for UUID

Addressed the CWE 340 by simply returning 400 when a numeric ID is given:

_user = search(Users.users, userId)
if _user:
    ret_object = {}
    ret_object.update({"uid": _user["uid"], "name": _user["name"]})
    return ret_object, 200
if Users.users.get(int(userId)):
    return "Forbidden", 400
else:
    return "Not found", 404

This solution took extra measure to limit the return fields. A safe serializer whitelists the expected fields to prevent unnecessary disclosure of sensitive data. A bad practice is to return the object without specifying which fields to serliaze in the response. This can disclose too much data. A good defensive practice is to explicitly define what the method should return (and it may require a bit of extra code).

Rate limiting was not really fixed but the player managed to pass the tests:

def check_rate_limit(userId):
    requests.pop(0)
    requests.append(userId)
    if (requests[0] == requests[1]) and (requests[1] == requests[2]):
        return True
    return False

Solution II

Pros:

• An explicit check for UUID
• Enforcement of UUID lookup

Cons:

• Hacky rate limiter

The player hardened the search method and introduced a new method to only allow UUID identifiers.

The UUID check was good because it enforced “what the API expects to receive”.

We can improve this UUID check by testing if it is a safe UUID.

def is_valid_uuid(value):
    try:
        uuid.UUID(str(value))
        return True
    except ValueError:
        return False
# SNIP
def search(values, searchFor):
    for index in values:
        if searchFor == values[index]["uid"]:
            return values[index]
    return None

For rate limiting the patch was hacky:

# This is so ugly :D
last_check = 0  # int(time.time())
allowance = 0
rate = 2.0
per  = 1.0  # seconds
allowance = rate

def ratelimit(msg_time):
    global allowance
    global last_check
    global rate
    global per
    time_passed = msg_time - last_check
    last_check = msg_time
    allowance += time_passed * (rate / per);
    if allowance > rate:
        allowance = rate
    if allowance < 1.0:
        return True
    allowance -= 1.0

It has a logic for rate limiting but it will not function in real scenarios. Firstly, it missed the source IP check and secondly, it missed the burst limit. Burst limit is the number of requests that are allowed before the limit is enforced.

Solution III: The winner

Pros:

• A simple rate limiter
• Enforcing UUID lookup
• Disallowing numeric user identifiers

Cons:

• Explicit UUID check

This solution was selected as the winner.

last_req = {}

def is_rate_limited(userId: str):
    rate_key = request.remote_addr
    if rate_key in last_req:
        if (datetime.now() - last_req[rate_key][0]).total_seconds() < 1:
            if last_req[rate_key][1] > 1:
                return True
            else:
                last_req[rate_key] = (datetime.now(), last_req[rate_key][1]+1)
        else:
            last_req[rate_key] = (datetime.now(), 1)    
    else:
        last_req[rate_key] = (datetime.now(), 1)
        
    return False
    

def get_user_detail(userId: str) -> Any:
    if userId.isnumeric():
        return "forbidden", 400
    try:
        _user = search(Users.users, userId)
        if _user:
            if is_rate_limited(userId):
                return "Too many requests", 429
            else:
                return _user, 200
    except Exception as ex:
        return {"message": str(ex)}, 404
    return "Not found", 404


def search(values, searchFor):
    for index in values:
        if searchFor == values[index]["uid"]:
            return values[index]
    return None

The rate limiting method has a rate key that consisted of a source IP and a burst counter. The Source IP was important otherwise, an adversary can create denial of service condition for all other users.

The search method is hardened. It only searches using uuid. The user ID check is removed and replaced with a new isnumeric() check.

This winning solution was submitted by mj1618 and received swags from us

Lessons learnt

Although the actual incident was assumed to be a trivial fix, in reality it takes more than a simple patch to address the root cause:

1. Authentication for API endpoints is necessary but do not rely on authentication as a solution. You may only hide the security vulnerability.
2. Arm API endpoints with rate limiting. Either implement it in your code by using a well-tested library or configure it in your web server/cloud service. Rate limiting can decrease the extent of an attack. Imagine in the actual incident, it could have slowed down the adversary.
3. Say no to enumerable identifiers, instead look into UUID.
4. Not all versions of UUIDs are random. Use UUID v4 and check if it is safely generated (More on this in our future challenges).
5. UUID should NOT be treated as a security control to protect identity of users. If your API only rely on UUID to provide secrecy (confidentiality), it has an insecure design. For example, do not use UUID as a password reset token! Although UUID v4 implementation should use a pseudo-random source, this is not guaranteed. UUID’s RFC discourages using it as a security control.
6. Be explicit about what your API returns by whitelisting fields that should get serialized.
7. Having multiple layers of security control is always better and this is a recommended defensive design pattern (why? see Swiss cheese model)
8. Finally, all the above security control should be enforced by the backend, i.e. the API. Frontend (Web UI, Mobile App UI, etc) checks are only good for UX and does not prevent adversaries.

Thanks to everyone who have attempted this challenge. We hope you have learnt something new. If you are an API developer, please consider the recommendations from this article for hardening your APIs.

If you would like to do security programming challenges, every month we host an online community security programming game event. You can join us at SecGames or you can try one of the many public challenges hosted on SecDim Play.