[GitHub] [cordova-android] brodybits commented on issue #740: Problem security audit and code correctness

[GitBox <gi...@apache.org>]

brodybits commented on issue #740: Problem security audit and code correctness
URL: https://github.com/apache/cordova-android/issues/740#issuecomment-503632221
 
 
   I would like to add a few more comments:
   
   A link to the code under discussion, which is in the `setBridgeMode` member function, would have been nice. Here it is: https://github.com/apache/cordova-android/blob/8.0.0/framework/src/org/apache/cordova/NativeToJsMessageQueue.java#L82-L100
   
   This link is tied to the most recent release tag (8.0.0) and shows the entire function block for the sake of context. And here is a link to the declaration of `bridgeModes`: https://github.com/apache/cordova-android/blob/8.0.0/framework/src/org/apache/cordova/NativeToJsMessageQueue.java#L55-L58
   
   I would consider the title to be a bit misleading. I cannot see how this could be a real security vulnerability at this point. I think it is also a bit unfortunate that the external tool labels this as a vulnerability with a priority level of "high". I think calling this a "potential vunerability" would have been more appropriate.
   
   Looking at the code, I think it does **not** match the [double-checked locking anti-pattern in Java ([1])](https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java) for the following reasons:
   
   - `newMode` is a local variable (I think that it is derived from an incoming argument or is used to set a non-local variable does not matter here)
   - we are not changing its value
   
   I do think it should be possible to make some improvements related to this code, such as:
   
   - move the synchronized part into a separate private synchronized function
   - declare `activeBridgeMode` as `@NonNull` and use a special noop bridge in case no native bridge is wanted
   
   [1] https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org