[Bug]: Incomplete URL substring sanitization Unvalidated Redirects and Forwards

Open
#735 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
45/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
python
Domain
cli, security

Research direction

Start with tests/integration/domains/test_slave_domains.py at line 73 and trace the URL handling it exercises. Check whether host validation relies on substring matching, then run the relevant integration test. Done means arbitrary hostname prefixes or embedded allowed-host text no longer pass validation while legitimate allowed hosts continue to work.

Written by the indexing model from the issue text.

Description

bug
CLI Version

v5.56.2

Command

https://github.com/linode/linode-cli/blob/fafe73e1f48a48ab9cbdf9b01f679e041f6bf3fa/tests/integration/domains/test_slave_domains.py#L73-L73

Sanitizing untrusted URLs is a common technique for preventing attacks such as request forgeries and malicious redirections. Usually, this is done by checking that the host of a URL is in a set of allowed hosts. However, treating the URL as a string and checking if one of the allowed hosts is a substring of the URL is very prone to errors. Malicious URLs can bypass such security checks by embedding one of the allowed hosts in an unexpected location.

Even if the substring check is not used in a security-critical context, the incomplete check may still cause undesirable behaviors when the check succeeds accidentally.

Output

No response

Expected Behavior

Recommendation

Parse a URL before performing a check on its host value, and ensure that the check handles arbitrary subdomain sequences correctly.

Actual Behavior

CWE-20

Steps to Reproduce

POC

The following code checks that a URL redirection will reach the example.com domain.

from flask import Flask, request, redirect
from urllib.parse import urlparse

app = Flask(__name__)

# Not safe, as "evil-example.net/example.com" would be accepted

@app.route('/some/path/bad1')
def unsafe1(request):
    target = request.args.get('target', '')
    if "example.com" in target:
        return redirect(target)

# Not safe, as "benign-looking-prefix-example.com" would be accepted

@app.route('/some/path/bad2')
def unsafe2(request):
    target = request.args.get('target', '')
    if target.endswith("example.com"):
        return redirect(target)



#Simplest and safest approach is to use an allowlist

@app.route('/some/path/good1')
def safe1(request):
    allowlist = [
        "example.com/home",
        "example.com/login",
    ]
    target = request.args.get('target', '')
    if target in allowlist:
        return redirect(target)

#More complex example allowing sub-domains.

@app.route('/some/path/good2')
def safe2(request):
    target = request.args.get('target', '')
    host = urlparse(target).hostname
    #Note the '.' preceding example.com
    if host and host.endswith(".example.com"):
        return redirect(target)

The first two examples show unsafe checks that are easily bypassed. In unsafe1 the attacker can simply add example.com anywhere in the url. For example, http://evil-example.net/example.com. In unsafe2 the attacker must use a hostname ending in example.com, but that is easy to do. For example, http://benign-looking-prefix-example.com.

The second two examples show safe checks. In safe1, an allowlist is used. Although fairly inflexible, this is easy to get right and is most likely to be safe. In safe2, urlparse is used to parse the URL, then the hostname is checked to make sure it ends with .example.com.

References

SSRF
XSS Unvalidated Redirects and Forwards Cheat Sheet.
CWE-20.

Dominant language
Python
Stars
442
Forks
159
Avg merge
7d 21h
Merged PRs (30d)
8

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from linode/linode-cli

All issues in linode/linode-cli

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.