[PR] Cordova-js: Event Listener Hijacking [cordova-js]

[via GitHub]

threatpointer opened a new pull request, #276:
URL: https://github.com/apache/cordova-js/pull/276

   ### Motivation and Context
   
   This issue was originially reported to secur...@apache.org.
   > the core issue remains the prevention of code injection: we do not 
consider any post-injection attack techniques as vulnerabilities. The change 
you propose may possibly be interesting as a security hardening improvement. 
For those, you don't need to use the private security reporting mechanisms, but 
you can use the regular open contribution workflow - you can find more 
information about that at https://cordova.apache.org/contribute/
   
   
   ####  About the Issue
   The current code modifies the default behavior of addEventListener and 
removeEventListener for both document and window objects to handle custom 
events. This modification can potentially be exploited if not properly secured. 
   
   #### [Relevant 
Code](https://github.com/apache/cordova-js/blob/ab52fd76714534d01fcb3773aa84c8c8d26e5a7f/src/cordova.js#L46
 )
   The manipulation of the document.addEventListener and 
window.addEventListener methods is crucial to the DOM event system. Overriding 
these methods allows directing certain events to custom documentEventHandlers 
and windowEventHandlers objects while allowing the default behavior for others. 
Malicious code (XSS) might modify these handler objects to include or replace 
handlers for specific events.
   
   #### Description of the Fix
   To mitigate such risks, we can encapsulate the modified addEventListener 
methods within an IIFE (Immediately Invoked Function Expression) to limit the 
scope and potential impact of the modifications. Additionally, checks are added 
to ensure that the subscribe function exists before calling it.
   
   ```
   `
   (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.includes(event.target.tagName.toLowerCase())) {
                           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
                   handler(event);
               };
               originalWindowAddEventListener.call(window, 'resize', 
secureHandler, false);
           }
       };
   })();
   
   
   
   `
   ```
   
   By implementing these practices, we can significantly reduce the risk of 
event listener hijacking. Having said that, if you beleive this should be left 
to the application to follow security best practices for input validation, CSP 
& loading of secure libraries of thrid party scripts and as a framework we 
shouldn't restrict such events. I am happy for you to close this PR with no 
action! This is more of defense in depth strategy to bake in security. 
   
   
   
   ### Testing
   Limited testing performed, could use some help here.


-- 
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