porscheofficial / cookie-consent-banner

The lightweight and flexible Cookie Consent Banner
MIT License
149 stars 6 forks source link

Absence of the consent-state cookie is not handled correctly #12

Closed diesieben07 closed 1 year ago

diesieben07 commented 1 year ago

Node/npm version(s): Node v18.13.0 NPM 8.19.3

Package(s) version(s):

    @porscheofficial/cookie-consent-banner@3.1.1

Browser(s) version(s): Chrome 111.0.5563.64

Description of the issue/feature: Consent banner does not open when cookie is absent, only when cookie is present, but set to the empty string. The cause of this is the following code (source):

const cookieValueString =
    `; ${document.cookie}`.split(`; ${this.cookieName}=`).pop() ??
    "".split(";").shift();

const cookieValues = cookieValueString ? cookieValueString.split(",") : [];

This code does not work properly if this.cookieName does not occur in document.cookie.

The first template string (`; ${document.cookie}`) can never be the empty string. Even if there are no cookies set at all and document.cookie is the empty string, `; ${document.cookie}` will be '; '. Calling split on a non-empty string will never produce an empty array, as such the following pop call will always return a value. As such anything past the ?? is never executed. Now, if document.cookie doesn't contain this.cookieName, then the result of pop will just be the entire document.cookie with ; prepended to it. This is then assumed to be the contents of the cookie and the code assumes a consent decision has already been made.

pboeder commented 1 year ago

Thanks for detailed analysis, @diesieben07! Will publish a new version in the next hours

diesieben07 commented 1 year ago

Thank you for the quick fix, however your solution only solves half of the problem. The code now checks whether document.cookie is completely empty, but this issue also arises (and does still arise in the new version) if there are other cookies set, but the consent state cookie is not present.