breautek commented on code in PR #276:
URL: https://github.com/apache/cordova-js/pull/276#discussion_r1705942511
##########
src/cordova.js:
##########
@@ -43,6 +43,80 @@ var m_window_removeEventListener =
window.removeEventListener;
var documentEventHandlers = {};
var windowEventHandlers = {};
+/**
+ * Mitigation for Event Listener Hijacking
+ */
+(function () {
+ var originalDocumentAddEventListener = document.addEventListener;
+ var originalWindowAddEventListener = window.addEventListener;
+ var documentEventHandlers = {};
+ var windowEventHandlers = {};
+
+ document.addEventListener = function (evt, handler, capture) {
+ var e = evt.toLowerCase();
+ if (typeof documentEventHandlers[e] !== 'undefined') {
+ if (typeof documentEventHandlers[e].subscribe === 'function') {
+ documentEventHandlers[e].subscribe(handler);
+ } else {
+ console.warn('No subscribe function defined for event:', e);
+ }
+ } else {
+ originalDocumentAddEventListener.call(document, evt, handler,
capture);
+ }
+ };
+
+ window.addEventListener = function (evt, handler, capture) {
+ var e = evt.toLowerCase();
+ if (typeof windowEventHandlers[e] !== 'undefined') {
+ if (typeof windowEventHandlers[e].subscribe === 'function') {
+ windowEventHandlers[e].subscribe(handler);
+ } else {
+ console.warn('No subscribe function defined for event:', e);
+ }
+ } else {
+ originalWindowAddEventListener.call(window, evt, handler, capture);
+ }
+ };
+
+ // Securely define your event handlers
+ documentEventHandlers.click = {
+ subscribe: function (handler) {
+ var secureHandler = function (event) {
+ // Perform necessary checks or actions before invoking the
handler
+ if (event && event.target) {
+ var allowedElements = ['button', 'a', 'div'];
+ if
(allowedElements.indexOf(event.target.tagName.toLowerCase()) > -1) {
+ handler(event);
+ } else {
+ console.warn('Click event handler ignored for
disallowed element:', event.target.tagName);
+ }
+ } else {
+ console.warn('Invalid event object in secure handler.');
+ }
+ };
+ originalDocumentAddEventListener.call(document, 'click',
secureHandler, false);
+ }
+ };
+
+ windowEventHandlers.resize = {
+ subscribe: function (handler) {
+ var secureHandler = function (event) {
+ // Perform necessary checks or actions before invoking the
handler
+ if (event && event.target) {
+ if (event.target === window) {
+ handler(event);
+ } else {
+ console.warn('Resize event handler ignored for
disallowed target:', event.target);
+ }
+ } else {
+ console.warn('Invalid event object in secure handler.');
+ }
+ };
+ originalWindowAddEventListener.call(window, 'resize',
secureHandler, false);
+ }
+ };
+})();
+
document.addEventListener = function (evt, handler, capture) {
var e = evt.toLowerCase();
Review Comment:
This assignment is duplicating an assignment done on line 55.
This version will use the globally accessible variable
`documentEventHandlers` declared on line 43 because it's out of scope of the
IIFE and will override the IIFE version declared on line 55, making your
changes redundant.
I believe the intent for this assignment to be replaced by the IIFE.
##########
src/cordova.js:
##########
@@ -43,6 +43,80 @@ var m_window_removeEventListener =
window.removeEventListener;
var documentEventHandlers = {};
var windowEventHandlers = {};
Review Comment:
These variables are being shadowed by the IIFE function, from lines 52 & 53.
I believe the intent for these to be removed since they are globally
accessible via the `window` namespace (since they are replaced by the IIFE
closure)
##########
src/cordova.js:
##########
@@ -43,6 +43,80 @@ var m_window_removeEventListener =
window.removeEventListener;
var documentEventHandlers = {};
var windowEventHandlers = {};
+/**
+ * Mitigation for Event Listener Hijacking
+ */
+(function () {
+ var originalDocumentAddEventListener = document.addEventListener;
+ var originalWindowAddEventListener = window.addEventListener;
+ var documentEventHandlers = {};
+ var windowEventHandlers = {};
+
+ document.addEventListener = function (evt, handler, capture) {
+ var e = evt.toLowerCase();
+ if (typeof documentEventHandlers[e] !== 'undefined') {
+ if (typeof documentEventHandlers[e].subscribe === 'function') {
+ documentEventHandlers[e].subscribe(handler);
+ } else {
+ console.warn('No subscribe function defined for event:', e);
+ }
+ } else {
+ originalDocumentAddEventListener.call(document, evt, handler,
capture);
+ }
+ };
+
+ window.addEventListener = function (evt, handler, capture) {
+ var e = evt.toLowerCase();
+ if (typeof windowEventHandlers[e] !== 'undefined') {
+ if (typeof windowEventHandlers[e].subscribe === 'function') {
+ windowEventHandlers[e].subscribe(handler);
+ } else {
+ console.warn('No subscribe function defined for event:', e);
+ }
+ } else {
+ originalWindowAddEventListener.call(window, evt, handler, capture);
+ }
+ };
+
+ // Securely define your event handlers
+ documentEventHandlers.click = {
+ subscribe: function (handler) {
+ var secureHandler = function (event) {
+ // Perform necessary checks or actions before invoking the
handler
+ if (event && event.target) {
+ var allowedElements = ['button', 'a', 'div'];
+ if
(allowedElements.indexOf(event.target.tagName.toLowerCase()) > -1) {
+ handler(event);
+ } else {
+ console.warn('Click event handler ignored for
disallowed element:', event.target.tagName);
+ }
+ } else {
+ console.warn('Invalid event object in secure handler.');
+ }
+ };
+ originalDocumentAddEventListener.call(document, 'click',
secureHandler, false);
+ }
+ };
+
+ windowEventHandlers.resize = {
+ subscribe: function (handler) {
+ var secureHandler = function (event) {
+ // Perform necessary checks or actions before invoking the
handler
+ if (event && event.target) {
+ if (event.target === window) {
+ handler(event);
+ } else {
+ console.warn('Resize event handler ignored for
disallowed target:', event.target);
+ }
+ } else {
+ console.warn('Invalid event object in secure handler.');
+ }
+ };
+ originalWindowAddEventListener.call(window, 'resize',
secureHandler, false);
+ }
+ };
+})();
+
document.addEventListener = function (evt, handler, capture) {
var e = evt.toLowerCase();
Review Comment:
I can't add another review block since it's too far away from modified code,
but the same applies for the `window.addEventListener` assignment on line 129,
which replaces the IIFE version declared on line 68.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org
