Skip to content

Commit 8b6b0fd

Browse files
authored
fix(security): remove process-global image cache and MCP resource interface (GHSA-25fp-988j-w29f) (#163)
utils.ad_creative_images was a module-level dict keyed by image hash alone, read by two registered MCP resources (meta-ads://resources enumerated every entry, meta-ads://images/{id} returned any entry's bytes) that never touched the request credential. Nothing had written to it since 0.4.0 — create_resource_from_image() held the only write and had no call sites — so the interface served an always-empty cache and no creative image was ever exposed through it. Removed rather than scoped, along with the dead writer and imports; regression tests assert none of it returns. Bumps to 1.0.123 and documents the advisory, including its reachability, in SECURITY.md.
1 parent 96eaf2d commit 8b6b0fd

11 files changed

Lines changed: 103 additions & 103 deletions

‎SECURITY.md‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,3 +221,40 @@ cache per caller would have kept the machinery without the benefit.
221221
typed tools instead: `get_ad_accounts`, `get_campaigns`,
222222
`get_campaign_details`, `get_adsets`, `get_ads`, `get_ad_details`.
223223
- Credited to [@BarakSrour](https://github.com/BarakSrour).
224+
225+
### GHSA-25fp-988j-w29f — MCP resource interface over a process-global, unscoped image cache
226+
227+
- **Severity:** Low (reported as Medium; see *Reachability* below)
228+
- **Affected versions:** `<= 1.0.122`
229+
- **Fixed in:** `1.0.123`
230+
- **Affected configurations:** Would have applied to multi-caller
231+
`--transport streamable-http` deployments. The hosted MCP at
232+
`*.mcp.pipeboard.co` does not expose the MCP resources interface to clients.
233+
234+
**What was wrong.** `utils.ad_creative_images` was a module-level dict of ad
235+
creative images keyed by image hash alone, with no caller, session or tenant
236+
component and no eviction. Two MCP resources registered on the server read it
237+
directly: `meta-ads://resources` (`list_resources`) enumerated every entry, and
238+
`meta-ads://images/{resource_id}` (`get_resource`) returned any entry's raw
239+
bytes. Neither handler resolved or used the request credential, so the shape of
240+
the code was a cross-caller read with an enumeration primitive attached.
241+
242+
**Reachability.** No shipped version could populate that cache. The only write
243+
was `create_resource_from_image()`, which has had no call sites since `0.4.0`
244+
(July 2025) — `get_ad_image` returns the image directly and never cached it.
245+
Verified against the running server: `meta-ads://resources` returns
246+
`{"resources": []}` and every `meta-ads://images/...` read returns
247+
"Resource not found". No creative image of any caller was ever exposed through
248+
this interface. The finding is accurate about the code and does not describe a
249+
disclosure that could occur in practice.
250+
251+
**Fix.** The cache, its writer, the `resources` module and both resource
252+
registrations were removed. An unscoped global with live readers is one commit
253+
away from being exploitable the moment a write path returns.
254+
255+
**Action for operators.** None required. Upgrade to `1.0.123` to drop the dead
256+
interface. Clients that enumerated `meta-ads://resources` (it only ever returned
257+
an empty list) should use `get_ad_image` / `get_ad_creatives` instead.
258+
259+
- Credited to [@Gal3m](https://github.com/Gal3m) and
260+
[@mohammad228](https://github.com/mohammad228).

‎meta_ads_mcp/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
from meta_ads_mcp.core.server import main
88

9-
__version__ = "1.0.122"
9+
__version__ = "1.0.123"
1010

1111
__all__ = [
1212
'get_ad_accounts',

‎meta_ads_mcp/core/ads.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ def _translate_video_customization_rules_for_existing_post(
433433
return translated_rules, videos_array
434434

435435

436-
from .utils import download_image, try_multiple_download_methods, ad_creative_images, extract_creative_image_urls
436+
from .utils import download_image, try_multiple_download_methods, extract_creative_image_urls
437437
from .server import mcp_server
438438

439439

‎meta_ads_mcp/core/insights.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import json
44
from typing import Optional, Union, Dict, List
55
from .api import meta_api_tool, make_api_request
6-
from .utils import download_image, try_multiple_download_methods, ad_creative_images, create_resource_from_image
6+
from .utils import download_image, try_multiple_download_methods
77
from .server import mcp_server
88
import base64
99
import datetime

‎meta_ads_mcp/core/resources.py‎

Lines changed: 0 additions & 46 deletions
This file was deleted.

‎meta_ads_mcp/core/server.py‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,11 @@
77
import json
88
from typing import Dict, Any, Optional
99
from .auth import login as login_auth
10-
from .resources import list_resources, get_resource
1110
from .utils import logger
1211

1312
# Initialize FastMCP server
1413
mcp_server = FastMCP("meta-ads")
1514

16-
# Register resource URIs
17-
mcp_server.resource(uri="meta-ads://resources")(list_resources)
18-
mcp_server.resource(uri="meta-ads://images/{resource_id}")(get_resource)
19-
2015

2116
class StreamableHTTPHandler:
2217
"""Handles stateless Streamable HTTP requests for Meta Ads MCP"""

‎meta_ads_mcp/core/utils.py‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,6 @@ def setup_logging():
7575
# Create the logger instance to be imported by other modules
7676
logger = setup_logging()
7777

78-
# Global store for ad creative images
79-
ad_creative_images = {}
80-
8178

8279
def extract_creative_image_urls(creative: Dict[str, Any]) -> List[str]:
8380
"""
@@ -369,28 +366,3 @@ async def try_multiple_download_methods(url: str) -> Optional[bytes]:
369366

370367
return None
371368

372-
373-
def create_resource_from_image(image_bytes: bytes, resource_id: str, name: str) -> Dict[str, Any]:
374-
"""
375-
Create a resource entry from image bytes.
376-
377-
Args:
378-
image_bytes: Raw image data
379-
resource_id: Unique identifier for the resource
380-
name: Human-readable name for the resource
381-
382-
Returns:
383-
Dictionary with resource information
384-
"""
385-
ad_creative_images[resource_id] = {
386-
"data": image_bytes,
387-
"mime_type": "image/jpeg",
388-
"name": name
389-
}
390-
391-
return {
392-
"resource_id": resource_id,
393-
"resource_uri": f"meta-ads://images/{resource_id}",
394-
"name": name,
395-
"size": len(image_bytes)
396-
}

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
44

55
[project]
66
name = "meta-ads-mcp"
7-
version = "1.0.122"
7+
version = "1.0.123"
88
description = "Model Context Protocol (MCP) server for Meta Ads - Use Remote MCP at pipeboard.co for easiest setup"
99
readme = "README.md"
1010
requires-python = ">=3.10"

‎server.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"$schema": "https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json",
33
"name": "co.pipeboard/meta-ads-mcp",
44
"description": "Facebook / Meta Ads automation with AI: analyze performance, test creatives, optimize spend.",
5-
"version": "1.0.122",
5+
"version": "1.0.123",
66
"remotes": [
77
{
88
"type": "streamable-http",

‎tests/test_get_ad_creatives_fix.py‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
Tests for issue where get_ad_creatives would throw:
44
'TypeError: 'dict' object is not callable'
55
6-
This was caused by trying to call ad_creative_images(creative) where
7-
ad_creative_images is a dictionary, not a function.
6+
This was caused by trying to call ad_creative_images(creative), which was a
7+
dictionary, not a function. That dictionary is gone (GHSA-25fp-988j-w29f).
88
99
The fix was to create extract_creative_image_urls() function and use that instead.
1010
"""
@@ -13,7 +13,6 @@
1313
import json
1414
from unittest.mock import AsyncMock, patch
1515
from meta_ads_mcp.core.ads import get_ad_creatives
16-
from meta_ads_mcp.core.utils import ad_creative_images
1716

1817

1918
@pytest.mark.asyncio
@@ -172,19 +171,3 @@ async def test_get_ad_creatives_strips_standard_enhancements(self):
172171
assert creative["creative_features_spec"]["image_touchups"]["enroll_status"] == "OPT_IN"
173172
assert "standard_enhancements" not in creative["degrees_of_freedom_spec"]["creative_features_spec"]
174173
assert creative["degrees_of_freedom_spec"]["creative_features_spec"]["profile_card"]["enroll_status"] == "OPT_IN"
175-
176-
177-
def test_ad_creative_images_is_dict():
178-
"""Test that ad_creative_images is a dictionary, not a function.
179-
180-
This confirms the original issue: ad_creative_images is a dict for storing image data,
181-
but was being called as a function ad_creative_images(creative), which would fail.
182-
This test ensures we don't accidentally change ad_creative_images to a function
183-
and break its intended purpose as a storage dictionary.
184-
"""
185-
assert isinstance(ad_creative_images, dict)
186-
187-
# This would raise TypeError: 'dict' object is not callable
188-
# This is the original bug - trying to call a dict as a function
189-
with pytest.raises(TypeError, match="'dict' object is not callable"):
190-
ad_creative_images({"test": "data"})

0 commit comments

Comments
 (0)