#22325: fix(security): prevent memory exhaustion in inline image decoding

## Summary Describe the problem and fix in 2–5 bullets: - **Problem:** MS Teams inbound attachment parsing is vulnerable to memory exhaustion (DoS). Inline `data:image/...;base64,...` content from HTML attachments is decoded directly into a Buffer before size limits are enforced. An attacker can send a large base64 payload that triggers massive allocations during `Buffer.from()` decode, causing high memory pressure or process instability. - **Why it matters:** This is a denial-of-service vector. An attacker can send specially crafted Teams messages with oversized inline images to exhaust process memory, crash the service, or degrade performance for legitimate users. - **What changed:** - Added `estimateBase64DecodedBytes()` and `isLikelyBase64Payload()` helpers to validate and estimate decoded size before allocation - Renamed `decodeDataImage()` → `decodeDataImageWithLimits()` to enforce per-image byte limits before `Buffer.from()` - Added `InlineImageLimitOptions` type for per-image and cumulative byte budgets - Updated `extractInlineImageCandidates()` to accept and enforce limits, tracking total estimated bytes - Connected limits in `downloadMSTeamsAttachments()` to pass byte constraints through the call path - **What did NOT change:** User-facing API behavior remains identical; limits enforcement is internal to security-sensitive parsing paths. ## Change Type (select all) - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Docs - [x] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [ ] Gateway / orchestration - [ ] Skills / tool execution - [ ] Auth / tokens - [x] Memory / storage - [x] Integrations - [ ] API / contracts - [ ] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes #[vulnerability-tracking-issue] - Related: MS Teams attachment security review ## User-visible / Behavior Changes None. The fix is purely internal DoS mitigation. Legitimate attachments (within configured byte limits) are processed identically. ## Security Impact (required) - New permissions/capabilities? **No** - Secrets/tokens handling changed? **No** - New/changed network calls? **No** - Command/tool execution surface changed? **No** - Data access scope changed? **No** **Explanation:** This is a *defensive* change that hardens parsing against memory exhaustion attacks. It adds pre-allocation size validation but does not expand attack surface. Untrusted inline base64 payloads are now constrained at parse time rather than after unbounded memory allocation. ## Repro + Verification ### Environment - OS: Any (vulnerability is runtime-independent) - Runtime/container: Node.js / container with TeamsAttachment handler - Integration/channel: MS Teams - Relevant config: `maxBytes` parameter to `downloadMSTeamsAttachments()` (existing) ### Steps **Before fix (vulnerable):** 1. Attacker sends Teams message with HTML attachment containing inline `data:image/png;base64,` + 1GB base64-encoded payload 2. Parser calls `decodeDataImage()` → `Buffer.from(payload, "base64")` 3. ~750MB memory allocated during decode before any checks 4. Process memory pressure/crash occurs if system memory exhausted **After fix (hardened):** 1. Same message arrives 2. Parser calls `decodeDataImageWithLimits()` with `maxInlineBytes: params.maxBytes` (e.g., 10MB) 3. `estimateBase64DecodedBytes()` returns estimate ~750MB 4. Check enforces: `if (estimatedBytes > opts.maxInlineBytes) return null` 5. Payload is rejected **before** `Buffer.from()` allocation 6. No memory pressure; safe rejection ### Expected - Large inline base64 payloads rejected at parse time with no memory spike - Legitimate inline images within configured limits accepted normally - Service continues processing without performance degradation ### Actual ✓ Verified in test scenarios; large payloads rejected before allocation ## Evidence - [x] Code inspection: Size checks validated before `Buffer.from()` in all paths - [x] Type safety verified: `decodeDataImageWithLimits()` return structure checked - [x] Limit flow traced: `downloadMSTeamsAttachments()` → `extractInlineImageCandidates(limits)` → `decodeDataImageWithLimits()` ## Human Verification (required) What you personally verified (not just CI), and how: - **Verified scenarios:** - Large base64 payloads (>configured limits) rejected before decode - Legitimate smaller payloads processed normally - Cumulative `maxInlineTotalBytes` limit enforced across multiple inline images - Error handling: invalid/non-base64 payloads filtered via `isLikelyBase64Payload()` - **Edge cases checked:** - Empty base64 payload → rejected (decoded size = 0) - Payload with whitespace/padding variations → estimated correctly - Payload at exact limit boundary → accepted - Payload 1 byte over limit → rejected - Multiple inline images hitting cumulative limit → stops processing at boundary - **What you did NOT verify:** - Cross-OS memory behavior (code is platform-independent) - Performance under high message throughput (separate perf testing) - Integration with other Teams channels (scope limited to attachment parsing) ## Compatibility / Migration - Backward compatible? **Yes** - Config/env changes? **No** - Migration needed? **No** The `limits` parameter to `extractInlineImageCandidates()` is optional; callers not passing it receive default behavior (no limits). Existing call sites must be updated to pass limits for protection (e.g., `downloadMSTeamsAttachments()` already does). ## Failure Recovery (if this breaks) - **How to disable/revert:** Remove `limits` argument from `extractInlineImageCandidates()` calls; revert to `shared.ts` and `download.ts` to prior versions without size validation - **Files to restore:** `extensions/msteams/src/attachments/shared.ts`, `extensions/msteams/src/attachments/download.ts` - **Bad symptoms to watch for:** - Legitimate inline images in Teams messages rejected unexpectedly (likely misconfigured `maxBytes`) - Parsing still slow/crashy with large attachments (size limits not threaded through all paths) - Type errors on `decodeDataImageWithLimits()` return structure (ensure callers handle both `candidate` and `estimatedBytes`) ## Risks and Mitigations - **Risk:** Misconfigured `maxBytes` limit too small, rejecting legitimate inline images - **Mitigation:** Use existing `params.maxBytes` passed to `downloadMSTeamsAttachments()` (already set for other attachments); audit limit value in production config - **Risk:** Estimation math error causes false rejections or false accepts - **Mitigation:** `estimateBase64DecodedBytes()` conservatively calculates `floor((len * 3) / 4) - padding`; verified against standard base64 decode formula - **Risk:** Attacker crafts non-base64 payload to bypass `isLikelyBase64Payload()` - **Mitigation:** Regex `^[A-Za-z0-9+/=\r\n]+$` is strict; invalid base64 still rejected in `Buffer.from()` catch block (defense-in-depth) - **Risk:** Cumulative limit tracking error across multiple inline images - **Mitigation:** Total tracked as `totalEstimatedInlineBytes`; boundary check uses estimated bytes before allocation, breaking loop at or before limit <!-- greptile_comment --> <h3>Greptile Summary</h3> Adds memory exhaustion protection to MS Teams inline image parsing by validating base64 payload sizes before allocation. The fix introduces `estimateBase64DecodedBytes()` to calculate decoded size upfront and `decodeDataImageWithLimits()` to enforce per-image and cumulative byte limits before calling `Buffer.from()`. The cumulative limit tracking correctly uses labeled `break outerLoop` to stop processing when total bytes exceed configured limits. The base64 size estimation formula `floor((len * 3) / 4) - padding` is mathematically correct and matches actual decoded sizes. Defense-in-depth is maintained with the redundant `byteLength` check after allocation. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - The security fix is well-designed and correctly implemented. The base64 estimation logic is mathematically sound, the cumulative limit tracking with labeled break is correct (previous issue was fixed), and the defense-in-depth approach with redundant checks provides additional safety. No functional bugs or security bypasses found. - No files require special attention _{Last reviewed commit: 8c32c61} <!-- greptile_other_comments_section --> <!-- /greptile_comment -->