-
Notifications
You must be signed in to change notification settings - Fork 154
# Bug Report: TokenHasBadFormatException when owner downloads archive #2516
Description
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:
- Log in as a user and upload files.
- Go to "My Transfers" and click on the transfer to view details.
- Click the "Download as ZIP" button.
- The error page appears: "A token that should allow access has a bad format."
Technical Analysis:
- Frontend: In
templates/transfer_detail_page.php,$archiveDownloadLinkis built using$token. For owners,$tokenis often undefined or initialized to empty (if patched). - Backend: In
www/download.php, the checkif(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.
- No Bypass: If a malicious user sends an empty token, the script skips the token validation block.
- Fallback to Session: The script immediately enters the
elseif(Auth::isAuthenticated())block. - 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
FileAccessDeniedExceptionis 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'])) {