Skip to content

Fix: Sanitize YouTube No-Cookie URL by Validating Hostname#1716

Open
apodacaduron wants to merge 1 commit into
sachinchoolur:masterfrom
apodacaduron:fix/youtube-nocookie-host-check
Open

Fix: Sanitize YouTube No-Cookie URL by Validating Hostname#1716
apodacaduron wants to merge 1 commit into
sachinchoolur:masterfrom
apodacaduron:fix/youtube-nocookie-host-check

Conversation

@apodacaduron
Copy link
Copy Markdown

Problem

The current implementation of isYouTubeNoCookie(url) uses a substring check:

return url.includes('youtube-nocookie.com');

This is unsafe because youtube-nocookie.com can appear anywhere in the string, including in malicious or misleading URLs such as: https://attacker.com/youtube-nocookie.com.fake.site
This would incorrectly pass the check, even though the host is not youtube-nocookie.com.

Solution

This PR improves the validation by parsing the URL with the URL constructor and comparing the host value directly:

var isYouTubeNoCookie = function (url) {
    try {
        var parsedUrl = new URL(url);
        return parsedUrl.host === 'youtube-nocookie.com';
    } catch (e) {
        return false; // Fails safely if the URL is invalid
    }
};

This ensures the function only returns true for valid URLs that have an exact hostname match.

Benefits

✅ Prevents false positives on malicious or incorrectly formatted URLs
✅ Improves security by relying on structured URL parsing
✅ Passes CodeQL analysis and aligns with secure coding practices
✅ Makes the plugin more robust and reliable

Replaces substring check in `isYouTubeNoCookie()` with explicit hostname validation
using the URL constructor. This prevents potential false positives or security risks
from malformed or malicious URLs.

Fix suggested by static analysis (CodeQL) and aligns with best practices for
URL sanitization.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 4, 2025

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants