[SECURITY]: Path Traversal (False Positive)

[PrimeObjects]

What happened?

My team is using https://snyk.io/ to scan the source codes in the LibreChat repo and we got a report below.

Issue: Path Traversal
Snyk Code: CWE-23 Definition
Snyk How-to: CWE-23 How-to

Version

v0.7.8

Screenshot

[danny-avila]

Hi @PrimeObjects thanks for the report. I can confirm this is a false positive.

The reported path traversal vulnerability is not exploitable due to proper input sanitization through the Multer middleware chain.

Security Flow

The file upload process follows this secure flow:

1. Route Definition

LibreChat/api/server/routes/files/index.js

Line 28 in 6488873

router.post('/images/avatar', upload.single('file'));

2. Multer Middleware

https://github.com/danny-avila/LibreChat/blob/6488873bad5117383a25bf1ea01adf3156342ea4/api/server/routes/files/multer.js

• The upload middleware is created by createMulterInstance()
• Uses multer.diskStorage with custom filename handling

3. Filename Sanitization

LibreChat/api/server/routes/files/multer.js

Lines 17 to 21 in 6488873

	filename: function (req, file, cb) {
	req.file_id = crypto.randomUUID();
	file.originalname = decodeURIComponent(file.originalname);
	const sanitizedFilename = sanitizeFilename(file.originalname);
	cb(null, sanitizedFilename);

4. sanitizeFilename Function

https://github.com/danny-avila/LibreChat/blob/6488873bad5117383a25bf1ea01adf3156342ea4/api/src/utils/files.ts

export function sanitizeFilename(inputName: string): string {
  // Remove any directory components
  let name = path.basename(inputName);
  
  // Replace any non-alphanumeric characters except for '.' and '-'
  name = name.replace(/[^a-zA-Z0-9.-]/g, '_');
  
  // Additional safety checks...
}

Key Security Controls

1. Path Traversal Prevention:

   • path.basename() strips all directory components (e.g., ../, ./, /etc/)
   • Any path traversal attempts like ../../../../etc/passwd become just passwd

2. Character Sanitization:

   • All special characters except . and - are replaced with _
   • This prevents injection of path separators or other malicious characters

3. Controlled Storage Location:

   • Files are stored in a user-specific directory: uploads/temp/{userId}/
   • The path is constructed server-side, not from user input

4. File Path Construction:

   • req.file.path is set by Multer using the sanitized filename
   • The path is: {uploads_dir}/temp/{user_id}/{sanitized_filename}

Test Coverage

The codebase includes comprehensive tests for these security controls:

1. Sanitization Tests

https://github.com/danny-avila/LibreChat/blob/6488873bad5117383a25bf1ea01adf3156342ea4/api/src/utils/files.spec.ts

• Tests removal of directory components
• Tests handling of special characters
• Tests edge cases (empty strings, dots, long filenames)

2. Multer Integration Tests

https://github.com/danny-avila/LibreChat/blob/6488873bad5117383a25bf1ea01adf3156342ea4/api/server/routes/files/multer.spec.js

• Tests filename sanitization in the middleware
• Tests handling of malformed inputs
• Tests path traversal attempts

Proof of Mitigation

When a user attempts to upload a file with a malicious filename:

Input: "../../../../etc/passwd"
After path.basename(): "passwd"
After sanitization: "passwd"
Final path: "/uploads/temp/user123/passwd"

The fs.readFile(req.file.path) call on line 381 is safe because:

1. req.file.path is generated by Multer, not user input
2. The filename component has been sanitized
3. The directory structure is server-controlled

Conclusion

The static analysis tool incorrectly flagged this as a vulnerability because it didn't trace the full data flow through the Multer middleware. The combination of:

1. Multer's controlled file storage
2. The sanitizeFilename function with path.basename()
3. Server-side path construction

Makes this implementation secure against path traversal attacks.

[PrimeObjects]

Hi Danny,

Thank you so much for your detailed responses — they address the issue perfectly.

There are a few additional "Path Traversal" issues flagged by Snyk (please see the attached screenshots). My initial impression is that many of these could likely be addressed with similar reasoning as before. Could you please review them and let me know if it’s appropriate to apply the same justification? If there are any exceptions, I’d appreciate your help in drafting specific responses for those cases.

Thanks a lot
Gary

Screenshots

[danny-avila]

Yes, all of these services/controllers use the same middleware via their respective routes, which is what initializes the file as req.file

https://github.com/danny-avila/LibreChat/blob/main/api/server/routes/files/index.js

[PrimeObjects]

I have "ignored" (in SNYK) all the issues in this "Path Traversal" category. I put the reason with the url of this discussion.