Code review with secureannex #106
FrancescoFaenzi
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
I asked secureannex to perform a review: https://app.secureannex.com/extensions/search/kngiafgkdnlkgmefdafaibkibegkcaef
For easier review I paste it here:
Code Review
Reanalyze
▼
background.js
1010 | FIREBASE_PROJECT_ID_DEV: "mcpsuperassistant-extenstion",
1011 | FIREBASE_API_KEY_DEV: "AIzaSyDaGAPP0o190Y2zfnngL_5_oerP_H8eRAw",
1012 | FIREBASE_APP_ID_DEV: "1:617083395344:web:4c7cc801de362934159995",
1013 | FIREBASE_PROJECT_ID_PROD: "mcpsuperassistant-extenstion",
1014 | FIREBASE_API_KEY_PROD: "AIzaSyDaGAPP0o190Y2zfnngL_5_oerP_H8eRAw",
1015 | FIREBASE_APP_ID_PROD: "1:617083395344:web:4c7cc801de362934159995"
Firebase API keys, project IDs, and app IDs are exposed in the code. This could allow unauthorized access or usage of Firebase resources.
1157 | if (v && typeof v == "string") {
1158 | const g = JSON.parse(v);
1159 | m = JSON.stringify(g, null, 2);
1160 | }
1161 | } catch {}
JSON parsing errors are silently ignored due to an empty catch block. This could lead to unexpected behavior if the string value is not valid JSON.
1174 | return this.cachedConfig[e] ? this.cachedConfig[e] : this.defaultConfig[e] ? {
1175 | value: this.defaultConfig[e],
1176 | source: "default"
1177 | } : {
1178 | value: "",
1179 | source: "static"
1180 | };
Returning a default value with source: "static" if a key is not found could be exploited if the application relies on the source property to determine the origin of the configuration value.
content/index.iife.js
197 | typeof process < "u" && Zs && Zs.NODE_ENV === "development" && typeof window < "u" && (window
198 | .__eventBus = ue, window.__eventBusDebug = () => ue.debugInfo());
The internal EventBus instance and a debugging function are exposed to the window object in development environments. If this code runs in production, it could allow malicious scripts to access and manipulate the EventBus.
375 | async attemptSendMessage (s, n, r, l = 5e3) {
376 | const f = this.generateMessageId(),
377 | p = {
378 | type: n,
379 | payload: r,
380 | origin: "content",
381 | timestamp: Date.now(),
382 | id: f
383 | };
384 | return this.config.enableLogging && console.debug("[ContextBridge] Sending message:", p,
385 | "to:", s), new Promise((v, S) => {
386 | const w = setTimeout(() => {
387 | this.pendingRequests.delete(f), S(new Error(
388 |
Message timeout after ${l}ms for ${n} to ${s}))389 | }, l);
390 | this.pendingRequests.set(f, {
391 | resolve: v,
392 | reject: S,
393 | timeout: w
394 | });
395 | try {
396 | if (!chrome.runtime || !chrome.runtime.sendMessage) throw new Error(
397 | "Chrome runtime not available");
398 | s === "background" ? chrome.runtime.sendMessage({
399 | ...p,
400 | expectResponse: !0
401 | }, A => {
402 | if (clearTimeout(w), this.pendingRequests.delete(f), chrome.runtime
403 | .lastError) {
404 | const _ =
Chrome runtime error: ${chrome.runtime.lastError.message};405 | this.config.enableLogging && console.error("[ContextBridge]", ), S(
406 | new Error());
407 | return
408 | }
409 | A ? A.error ? S(new Error(A.error)) : v(A.payload !== void 0 ? A.payload :
410 | A) : v(null)
411 | }) : chrome.runtime.sendMessage({
412 | ...p,
413 | target: s,
414 | expectResponse: !0
415 | }, A => {
416 | if (clearTimeout(w), this.pendingRequests.delete(f), chrome.runtime
417 | .lastError) {
418 | const _ =
Chrome runtime error: ${chrome.runtime.lastError.message};419 | this.config.enableLogging && console.error("[ContextBridge]", ), S(
420 | new Error());
421 | return
422 | }
423 | A ? A.error ? S(new Error(A.error)) : v(A.payload !== void 0 ? A.payload :
424 | A) : v(null)
425 | })
426 | } catch (A) {
427 | clearTimeout(w), this.pendingRequests.delete(f), A instanceof Error && A.message
428 | .includes("Extension context invalidated") && (this.isExtensionContextValid = !
429 | 1, ue.emit("context:bridge-invalidated", {
430 | timestamp: Date.now(),
431 | error: A.message
432 | })), this.config.enableLogging && console.error(
433 | "[ContextBridge] Error sending message:", A), S(A)
434 | }
435 | })
436 | }
The attemptSendMessage function lacks target validation, allowing a malicious website to specify an arbitrary target and potentially cause the content script to send messages to unintended origins.
What do you think?
Beta Was this translation helpful? Give feedback.
All reactions