Prototype Pollution in alvarotrigo/fullpage.js

Valid

Reported on

Feb 26th 2022


Description

fullPage utils are available to developers using window.fp_utils. They can use these utils for their own use-case (other than fullPage) as well. However, one of the utils deepExtend is vulnerable to Prototype Pollution vulnerability.

Javascript is "prototype" language which means when a new "object" is created, it carries the predefined properties and methods of an "object" with itself like toString, constructor etc. By using prototype-pollution vulnerability, an attacker can overwrite/create the property of that "object" type. If the victim developer has used that property anywhere in the code, then it will have severe effect on the application.

For e.g.:

var obj = {};
console.log(obj.A); // undefined
obj["__proto__"].A = 1;
console.log(obj.A);  // 1
var new_obj = {};
console.log(new_obj.A); // 1  -> exploit

Proof of Concept

STEP 1: Visit https://alvarotrigo.com/fullPage demo.

STEP 2: Run the following code in dev tools console

NOTE: I am asking to run this in console for PoC purpose only. The real-world exploitation scenario may vary.

var o = {};
o.toString();
var obj = window.fp_utils.deepExtend({},{"constructor": {"prototype": {"toString": () => {alert(`XSS via Prototype Pollution`)}}}});

STEP 3: Call toString prototype function

o.toString();

and you will see an alert pop-up showing XSS exploitation.

Impact

Prototype pollution can be used to create/overwrite predefined properties and methods of object type. It can lead to XSS, change code logic, DoS etc. based on the application code.

We are processing your report and will contact the alvarotrigo/fullpage.js team within 24 hours. a year ago
We created a GitHub Issue asking the maintainers to create a SECURITY.md a year ago
Álvaro
a year ago

Maintainer


Thanks for that!

Replacing the deepExtend function for the following one seems to fix the bug:

 function deepExtend( a, b ) {
    a = a || {};

    for ( var prop in b ) {
      a[ prop ] = b[ prop ];
    }
    return a;
}

I'll be releasing fullPage.js version 4 this week with this change (and many others)

Álvaro
a year ago

Maintainer


Ok, this function won't do exactly the same, It will fail to merge deep objects.

Any proposed solution to this vulnerability?

Álvaro
a year ago

Maintainer


Ok, Seeing the references I did this and it seems to have solve it: https://jsfiddle.net/7c9uo3yn/

if (!obj.hasOwnProperty(key) || key == '__proto__'  || key == 'constructor'){
                continue;
}

Although... I'm not sure if I should do:

key == '__proto__' 

Or

key == 'prototype'

Any hints on that?

Rohan Sharma
a year ago

Researcher


yes, as mentioned. you just need to check for keys __proto__ and constructor

Rohan Sharma
a year ago

Researcher


no need to check for prototype

Jamie Slome
a year ago

Admin


@maintainer - would you kindly mark the report as valid and confirm the fix now that a patch has been released in v4?

You can do these actions using the dropdown below ⬇️

We have contacted a member of the alvarotrigo/fullpage.js team and are waiting to hear back a year ago
Álvaro validated this vulnerability a year ago
Rohan Sharma has been awarded the disclosure bounty
The fix bounty is now up for grabs
Álvaro marked this as fixed in 4.0.2 with commit bf6249 a year ago
Álvaro has been awarded the fix bounty
This vulnerability will not receive a CVE
Álvaro
a year ago

Maintainer


Thanks for reporting it! ;) The issue has been fixed in version 4 (4.0.2 is the latest)

Jamie Slome
a year ago

Admin


Great work all! 👍

to join this conversation