Skip to content

# Bug Report: TokenHasBadFormatException when owner downloads archive #2516

@victoritis

Description

@victoritis

Title: TokenHasBadFormatException when owner downloads archive from transfer detail page

System:

  • FileSender Version: 3.3 / Master
  • PHP Version: 8.x

Description:
When a transfer owner tries to download all files as an archive (ZIP/TAR) from the Transfer Detail page (?s=transfer_detail&transfer_id=X), the application throws a TokenHasBadFormatException.

This happens because the archive download form generation in templates/transfer_detail_page.php uses an undefined or empty $token variable when the user is the owner (accessing via transfer_detail).

Consequently, when the form is submitted to download.php, it sends token= (empty string). The logic in download.php checks array_key_exists('token', $_REQUEST), which returns true for empty parameters, and subsequently calls Utilities::isValidUID($token) on the empty string, causing the exception.

Steps to reproduce:

  1. Log in as a user and upload files.
  2. Go to "My Transfers" and click on the transfer to view details.
  3. Click the "Download as ZIP" button.
  4. The error page appears: "A token that should allow access has a bad format."

Technical Analysis:

  1. Frontend: In templates/transfer_detail_page.php, $archiveDownloadLink is built using $token. For owners, $token is often undefined or initialized to empty (if patched).
  2. Backend: In www/download.php, the check if(array_key_exists('token', $_REQUEST)) triggers even for empty tokens.
    if(array_key_exists('token', $_REQUEST)) {
        $token = $_REQUEST['token'];
        if(!Utilities::isValidUID($token)) // <--- Fails here because "" is not a valid UID
           throw new TokenHasBadFormatException($token);
    // ...
    }

Suggested Fix:
In www/download.php, change the condition to ignore empty tokens. This allows the script to fall through to the elseif(Auth::isAuthenticated()) block, which correctly handles authentication for transfer owners.

Detailed Logic & Solution:

// Step 1: Check if "token" parameter exists in URL (and is NOT empty)
// CHANGE: array_key_exists('token', $_REQUEST) -> !empty($_REQUEST['token'])
if(!empty($_REQUEST['token'])) {
    $token = $_REQUEST['token'];
    
    // Step 2: Validate token format
    // This previously failed for owners because token was empty string ""
    if(!Utilities::isValidUID($token))
        throw new TokenHasBadFormatException($token);
    
    // (Authorization granted for valid recipient token...)
    
// Step 3: If NO token (or empty with this fix), check if authenticated
} elseif(Auth::isAuthenticated()) {
    // Check if user is Admin or Owner
    if(!Auth::isAdmin()) {
        if(!Auth::user()->is(File::fromId((int)$fid)->transfer->owner))
             throw new FileAccessDeniedException($fid);
    }
    // (Authorization granted for Owner)
}

Security Analysis:
This change is safe because it does not bypass authentication. It merely treats an empty token string (token=) the same as a missing token parameter.

  1. No Bypass: If a malicious user sends an empty token, the script skips the token validation block.
  2. Fallback to Session: The script immediately enters the elseif(Auth::isAuthenticated()) block.
  3. Strict Validation: Inside this block, the system enforces that the user must be logged in AND must be the legitimate owner of the file (or an Admin). If these conditions are not met, a FileAccessDeniedException is thrown.
    Therefore, skipping the "Bad Format" exception for empty tokens does not open any security hole; it correctly delegates access control to the session-based owner validation logic.

Patch:

// www/download.php around line 59

// Change:
if(array_key_exists('token', $_REQUEST)) {

// To:
if(!empty($_REQUEST['token'])) {

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions