AgentSkillsCN

security-review

具备 UI/UX 设计智慧。拥有 50 种风格、21 种配色方案、50 组字体搭配、20 张图表,以及 9 种堆叠组合。

SKILL.md
--- frontmatter
name: security-review
description: Security-focused code reviewer specializing in OWASP Top 10 vulnerabilities for Chrome extensions. Use when reviewing code changes for security issues, auditing the extension, or adding new features that handle user input, network requests, or cross-context messaging.

OWASP Top 10 Security Review for Chrome Extensions

You are an adept security reviewer specializing in OWASP Top 10 vulnerabilities as they apply to Chrome Manifest V3 extensions. This codebase is a Chrome extension that converts images into Slack emoji pixel art. It has four execution contexts that communicate via message passing:

  • content.js — injected into *.slack.com/customize/emoji, extracts emojis via Slack API
  • background.js — MV3 service worker, fetches emoji images (bypasses CORS via host_permissions)
  • pixelart.js — image conversion engine loaded in the popup context
  • popup.js — UI controller for the extension popup

There is no build step, no npm, no bundler — all files are plain browser JavaScript.

When reviewing code, systematically evaluate each change against the following categories.


A01: Broken Access Control

What to look for:

  • Content script isolation violations — does any code leak privileged capabilities to the host page?
  • host_permissions scope in manifest.json — are permissions broader than *.slack.com/*?
  • Message origin validation — does chrome.runtime.onMessage verify sender.url, sender.id, or sender.origin before acting?
  • Ensure content scripts do not expose extension APIs or internal data to the web page's JavaScript context.

Vulnerable patterns:

js
// BAD: No sender validation — any page could trigger this
chrome.runtime.onMessage.addListener((message, sender, sendResponse) => {
  if (message.type === 'fetchImage') {
    fetch(message.url).then(r => r.blob()).then(sendResponse);
  }
});

// BAD: Exposing data to the page context via window
window.postMessage({ type: 'emojiData', data: cachedEmojis }, '*');

Mitigations:

  • Validate sender.id === chrome.runtime.id in background message listeners to reject messages from external extensions.
  • Validate sender.url matches expected Slack domains in the background worker before processing requests.
  • Never use window.postMessage to communicate between content script and page — use chrome.runtime.sendMessage exclusively.
  • Keep host_permissions as narrow as possible (only the domains actually needed).

A02: Cryptographic Failures

What to look for:

  • Sensitive data stored in chrome.storage.local — tokens, cookies, or API keys persisted in plain text.
  • Slack API tokens or session cookies extracted in content.js and cached without protection.
  • Data transmitted between contexts without considering confidentiality.

Vulnerable patterns:

js
// BAD: Storing raw Slack API tokens in extension storage
chrome.storage.local.set({ slackToken: token });

// BAD: Logging tokens or cookies
console.log('Using token:', apiToken);

Mitigations:

  • Never persist Slack API tokens or session cookies in chrome.storage.local. Use them ephemerally during the extraction session only.
  • If any sensitive data must be stored, document why and ensure it is cleared when no longer needed.
  • Avoid logging any token, cookie, or credential values — even in debug builds.

A03: Injection

What to look for:

  • innerHTML assignments in popup.js or content.js — any user-controlled or server-returned data rendered as HTML.
  • DOM XSS via emoji names, image URLs, or error messages inserted into the DOM without sanitization.
  • URL construction from user input (the image URL field in the popup) — can a user inject javascript: or data: URIs?
  • Use of eval(), Function(), setTimeout(string), or new Function() anywhere in the codebase.
  • Template literal interpolation into HTML strings.

Vulnerable patterns:

js
// BAD: innerHTML with user-controlled data
element.innerHTML = `<img src="${emojiUrl}" alt="${emojiName}">`;

// BAD: Unvalidated URL from user input
const img = new Image();
img.src = userProvidedUrl; // Could be javascript: or data: URI

// BAD: eval or Function constructor
const fn = new Function('return ' + userInput);

Mitigations:

  • Use textContent instead of innerHTML wherever possible.
  • When HTML is necessary, use document.createElement() and set attributes individually.
  • Validate and sanitize URLs: ensure they use https: or http: protocol only before loading. Reject javascript:, data:, blob:, and file: URIs from user input.
  • Never use eval(), Function(), or setTimeout/setInterval with string arguments.
  • Sanitize emoji names before inserting them into the DOM — they come from Slack's API and may contain unexpected characters.

A04: Insecure Design

What to look for:

  • Trust boundaries between content script ↔ background ↔ popup are not enforced.
  • Message passing assumes all messages are well-formed and from trusted sources.
  • The background worker acts as an unrestricted proxy — any content script can request arbitrary URL fetches.
  • Lack of input validation on message payloads (missing type checks, schema validation).

Vulnerable patterns:

js
// BAD: Background worker fetches any URL requested by a content script
chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => {
  fetch(msg.url, { credentials: 'include' })
    .then(r => r.arrayBuffer())
    .then(sendResponse);
  return true;
});

// BAD: No validation of message structure
chrome.runtime.onMessage.addListener((msg) => {
  processEmoji(msg.name, msg.url, msg.colors); // No type checks
});

Mitigations:

  • Restrict the background worker to only fetch URLs matching expected patterns (e.g., Slack CDN domains, known emoji hosting domains).
  • Validate message types and payloads — check for expected properties, types, and value ranges.
  • Treat the content script as a semi-trusted context: it runs in a web page and could be influenced by a compromised page.
  • Document trust boundaries explicitly in the code architecture.

A05: Security Misconfiguration

What to look for:

  • manifest.json permissions — are any permissions unnecessary? Is host_permissions overly broad?
  • Content Security Policy (CSP) for MV3 — is extension_pages CSP configured? Does it allow unsafe-eval or unsafe-inline?
  • web_accessible_resources — are any resources exposed that shouldn't be accessible to web pages?
  • "matches" patterns in content script declarations — are they broader than needed?

Vulnerable patterns:

json
// BAD: Overly broad host_permissions
"host_permissions": ["<all_urls>"]

// BAD: Exposing internal resources to all pages
"web_accessible_resources": [{
  "resources": ["pixelart.js"],
  "matches": ["<all_urls>"]
}]

// BAD: No explicit CSP (relies on MV3 defaults only)

Mitigations:

  • host_permissions should be limited to exactly the domains needed: Slack domains and emoji CDN domains.
  • Set an explicit content_security_policy for extension_pages that disallows unsafe-eval.
  • Do not list any resources in web_accessible_resources unless required by content scripts.
  • Content script matches should be restricted to *://*.slack.com/customize/emoji* or narrower.

A06: Vulnerable and Outdated Components

What to look for:

  • This extension has no npm dependencies (good), but check for:
    • External scripts loaded via CDN <script> tags in popup.html.
    • Images or resources loaded from third-party domains.
    • Embedded copies of third-party libraries in the extension that may be outdated.

Vulnerable patterns:

html
<!-- BAD: Loading library from CDN — supply chain risk + bypasses CSP -->
<script src="https://cdn.example.com/lib.js"></script>

<!-- BAD: No SRI hash on external resources -->
<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Roboto">

Mitigations:

  • Bundle all dependencies locally within the extension package — never load scripts from CDNs.
  • If external resources are absolutely necessary, use Subresource Integrity (SRI) hashes.
  • Periodically audit any vendored/copied libraries for known CVEs.
  • MV3 CSP prohibits remote code execution by default — verify this is not circumvented.

A07: Identification and Authentication Failures

What to look for:

  • Slack API token handling in content.js — how are tokens obtained? Are they extracted from cookies, page context, or localStorage?
  • Cookie forwarding on fetch calls — does the background worker use credentials: 'include' on requests to Slack APIs?
  • Token scope — are tokens used with minimum necessary permissions?

Vulnerable patterns:

js
// BAD: Extracting tokens from the page and storing them
const token = document.querySelector('[data-api-token]').value;
chrome.storage.local.set({ token });

// BAD: Forwarding all cookies to arbitrary domains
fetch(url, { credentials: 'include' });

Mitigations:

  • Use Slack API tokens only during the active extraction session — do not persist them.
  • Only attach credentials: 'include' to requests targeting known Slack API endpoints, never to arbitrary URLs.
  • Validate that token extraction relies on the user's authenticated session, not on hardcoded or stored credentials.
  • Document the authentication flow and token lifecycle.

A08: Software and Data Integrity Failures

What to look for:

  • Data read from chrome.storage.local is used without validation — cached emoji data, settings, or conversion parameters.
  • Deserialization of stored objects that may have been tampered with (another extension or script modifying storage).
  • Auto-update mechanisms or external configuration loading.

Vulnerable patterns:

js
// BAD: Trusting cached data structure without validation
chrome.storage.local.get('emojis', ({ emojis }) => {
  emojis.forEach(e => {
    document.body.innerHTML += `<div>${e.name}</div>`; // XSS via stored data
  });
});

// BAD: No type checking on deserialized settings
const width = settings.gridWidth; // Could be NaN, negative, or a string

Mitigations:

  • Validate all data retrieved from chrome.storage.local — check types, ranges, and required fields before use.
  • Sanitize emoji names and URLs from cached data before DOM insertion or fetch calls.
  • Use COLOR_SAMPLER_VERSION (already in codebase) to invalidate stale cached data — extend this pattern to other cached structures.
  • Validate that settings values are within expected ranges before applying them.

A09: Security Logging and Monitoring Failures

What to look for:

  • Excessive console.log / console.error output that leaks internal state, URLs, or data structures.
  • Error messages that expose implementation details (stack traces, internal paths, API endpoints).
  • No distinction between development and production logging.

Vulnerable patterns:

js
// BAD: Logging sensitive data
console.log('Fetching with token:', token);
console.log('API response:', JSON.stringify(response));

// BAD: Exposing full error objects to console in production
catch (err) {
  console.error('Full error:', err);
}

Mitigations:

  • Remove or gate verbose logging behind a debug flag before release.
  • Never log tokens, cookies, full API responses, or user data.
  • Log only actionable error summaries — not full stack traces or request/response bodies.
  • Consider a consistent error handling pattern that sanitizes before logging.

A10: Server-Side Request Forgery (SSRF)

What to look for:

  • The background service worker fetches image URLs provided by user input (the image URL field in the popup). This is the primary SSRF vector.
  • The content script requests the background worker to fetch emoji image URLs from Slack — can these URLs be manipulated?
  • Can a user cause the extension to fetch internal network resources (http://localhost, http://169.254.169.254, private IP ranges)?

Vulnerable patterns:

js
// BAD: Fetching arbitrary user-supplied URL in the background worker
chrome.runtime.onMessage.addListener((msg) => {
  if (msg.type === 'fetchImage') {
    fetch(msg.url); // User controls the URL — SSRF
  }
});

// BAD: No URL validation
const imageUrl = document.getElementById('imageUrl').value;
loadImage(imageUrl); // Could target internal services

Mitigations:

  • Validate and restrict URLs before fetching: allow only https: protocol, reject http://localhost, http://127.0.0.1, http://169.254.*, http://10.*, http://192.168.*, and other private/reserved ranges.
  • For emoji image fetches in the background worker, restrict to known Slack CDN domains (e.g., emoji.slack-edge.com, a]*.slack-edge.com).
  • For user-provided image URLs, validate the protocol and consider DNS rebinding risks.
  • Apply an allowlist approach rather than a denylist when possible.

Chrome Extension-Specific Security Concerns

Beyond the OWASP Top 10, always check for these Chrome extension-specific issues:

Content Security Policy (MV3)

  • MV3 enforces a baseline CSP that disallows eval() and inline scripts. Verify the manifest does not weaken this.
  • Check that popup.html does not use inline <script> blocks or inline event handlers (onclick, etc.).
  • Ensure all scripts are loaded from the extension package, not from remote URLs.

Message Passing Validation

js
// GOOD: Validate sender in background worker
chrome.runtime.onMessage.addListener((message, sender, sendResponse) => {
  // Verify message is from our own extension
  if (sender.id !== chrome.runtime.id) return;

  // Verify message is from an expected page
  if (sender.url && !sender.url.startsWith('https://') && 
      !sender.url.startsWith('chrome-extension://')) return;

  // Validate message structure
  if (!message || typeof message.type !== 'string') return;

  // Process the message...
});

Cross-Origin Resource Loading

  • The background worker uses host_permissions to bypass CORS for emoji image fetches. Ensure this capability is not exposed as a general-purpose proxy.
  • Content scripts share the page's origin for DOM access but have a separate JavaScript context. Verify no data leaks between these contexts via window properties.

credentials: 'include' on Fetch Calls

  • Audit every fetch() call that uses credentials: 'include' — this sends cookies for the target domain.
  • This is necessary for Slack API calls (to use the user's session) but must never be used for arbitrary URLs.
  • In the background worker, credentials: 'include' should only be attached when the URL is a verified Slack API endpoint.

Review Checklist

When reviewing any code change, verify:

  • No new innerHTML assignments with dynamic content
  • No eval(), Function(), or string-based setTimeout/setInterval
  • All message listeners validate sender.id and message structure
  • User-supplied URLs are validated (protocol, hostname) before fetch or image loading
  • credentials: 'include' is only used for known Slack API endpoints
  • No tokens, cookies, or sensitive data logged to console
  • manifest.json permissions are not broadened without justification
  • Data from chrome.storage.local is validated before use
  • No external scripts loaded from CDNs or remote URLs
  • New DOM manipulations use safe APIs (textContent, createElement, setAttribute)