Back to skill

Security audit

HTTP Retry - HTTP 重试机制

Security checks for vulnerabilities and agentic risk

Overview

This skill is an HTTP retry helper, but its shipped code reports successful HTTP responses without actually making the request.

Review this carefully before installing or using it. It is not a reliable HTTP retry implementation as shipped: it can tell your program an HTTP request succeeded when no request occurred. Do not use it for authentication, payments, audit logging, data submission, policy checks, or any workflow where a real server response matters unless the transport callback, POST handling, validation, and tests are fixed.

Vulnerability Patterns
  • Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
  • Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
  • Agent Memory PoisoningWrites attacker-controlled rules into memory that affect later sessions
  • Remote Payload Retrieval and ExecutionFetches external code whose behavior can change after review
  • Embedded Malicious CodeShips malicious scripts inside the skill and executes them locally
Findings (2)

T09 · Insecure Skill Coding Practices

Error
Location
http_retry.h:96
Finding
Fabricated HTTP Success Responses Cause Fail-Open Behavior<![CDATA[ ## Vulnerability Details **File Location**: `http_retry.h`, lines 96-103 and 137-140 **Vulnerability Type**: Fail-open response fabrication **Risk Level**: High ### Vulnerable Code ```c // Execute request (placeholder - integrate with actual HTTP library) // HttpErrorCode error = config->request_func(url, config->user_context); // For demo purposes, simulate success HttpErrorCode error = HTTP_SUCCESS; response.status_code = 200; if (error == HTTP_SUCCESS) { ``` ```c HttpResponse http_post_retry(const char* url, const char* data) { // Similar to GET, but with POST method return http_get_retry(url); // Placeholder } ``` ### Technical Analysis The HTTP request callback is commented out and never invoked. Instead, the implementation unconditionally assigns `HTTP_SUCCESS` and status code `200`, irrespective of the URL, network state, server response, or configured callback. Consequently, the function does not perform an HTTP request but reports success to its caller. The POST wrapper also discards the `data` argument and delegates to the same stubbed GET implementation. The documented timeout, retry, connection-pool, rate-limit, and transient-error behavior is therefore not implemented. This is a fail-open integrity vulnerability. Software using this component may treat a required remote operation as completed even though no data was transmitted and no response was received. ### Attack Path 1. An application uses `http_request_with_retry()` or `http_post_retry()` for a security-relevant remote operation. 2. The operation may involve authentication, authorization, notification delivery, data upload, payment processing, audit logging, or another required server-side action. 3. The implementation does not invoke `config->request_func` and discards POST data. 4. It sets `error` to `HTTP_SUCCESS` and `status_code` to `200`. 5. The calling application receives a fabricated succes ...[truncated 773 chars]
Remediation
<![CDATA[ ## Remediation Suggestions 1. Invoke the configured transport callback instead of assigning a hardcoded success value. 2. Return an explicit configuration or unsupported-operation error when `request_func` is absent. 3. Require the transport layer to return the real status code, response body, response size, and normalized error code. 4. Implement separate method-aware behavior so that POST data is transmitted rather than discarded. 5. Apply the configured timeout to the actual network operation. 6. Retry only failures classified as transient, and preserve the final real error when all attempts fail. 7. Never synthesize status code `200` unless a transport implementation has received and validated that response. 8. Add tests proving that: - The callback is invoked. - URLs and request bodies reach the transport. - Connection failures cannot become successful responses. - Missing callbacks fail closed. - POST and GET operations retain their intended semantics. ]]>

T09 · Insecure Skill Coding Practices

Warning
Location
http_retry.h:59
Finding
Unchecked Retry Parameters Permit Undefined Behavior and Denial of Service<![CDATA[ ## Vulnerability Details **File Location**: `http_retry.h`, lines 59-64 and 119-120 **Vulnerability Type**: Integer overflow, invalid shift, modulo by zero, and unsafe delay conversion **Risk Level**: Medium ### Vulnerable Code ```c int calculate_backoff_delay(int attempt, int base_delay_ms, int max_delay_ms) { int exponential_delay = base_delay_ms * (1 << attempt); // 2^attempt int jitter = rand() % (exponential_delay / 2); int delay = exponential_delay + jitter; return (delay > max_delay_ms) ? max_delay_ms : delay; } ``` ```c int delay_ms = calculate_backoff_delay(attempt, config->base_delay_ms, config->max_delay_ms); usleep(delay_ms * 1000); ``` ### Technical Analysis All retry parameters are caller-controlled integers, but the implementation performs no range or sign validation. The expression `1 << attempt` has undefined behavior when `attempt` is negative, reaches or exceeds the bit width of `int`, or shifts into a nonrepresentable signed value. Multiplying the result by `base_delay_ms` can cause signed integer overflow. If `exponential_delay` is zero or one, `exponential_delay / 2` is zero, causing `rand() % 0`, which is undefined behavior and commonly terminates the process. This is directly reachable with configurations such as `base_delay_ms = 0`. The addition of jitter and the conversion `delay_ms * 1000` can also overflow. Negative values may subsequently be converted to the unsigned argument type expected by `usleep`, potentially producing an unexpectedly long delay. Excessive but valid configuration values can intentionally stall a worker for long periods. ### Attack Path 1. An attacker influences retry configuration directly or through application-controlled settings. 2. The attacker supplies a zero, negative, or extreme value for `base_delay_ms`, `max_delay_ms`, or `max_attempts`. 3. A retryable request failure causes the backoff path to execute. 4. The delay calculation performs an invalid sh ...[truncated 992 chars]
Remediation
<![CDATA[ ## Remediation Suggestions 1. Validate configuration before entering the retry loop: - Require `max_attempts` to be positive and enforce a conservative upper bound. - Require `base_delay_ms` and `max_delay_ms` to be positive. - Require `base_delay_ms <= max_delay_ms`. - Enforce an absolute maximum delay. 2. Use checked `uint64_t` arithmetic instead of signed `int` arithmetic for delay calculations. 3. Avoid unbounded left shifts. Either cap the exponent before shifting or calculate backoff with checked multiplication. 4. Handle the jitter range explicitly and never execute modulo with a zero divisor. 5. Clamp the delay before adding jitter and again before converting milliseconds to microseconds. 6. Use a sleep API with a structured duration, such as `nanosleep`, and handle interrupted sleeps safely. 7. Return a configuration error instead of attempting to normalize invalid negative or excessive values. 8. Add boundary tests for zero, one, negative, maximum-width, overflowing, and excessively large parameter values. ]]>
Vulnerability Patterns
  • Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
  • MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
  • Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
  • Privilege EscalationExcessive Permissions, Sudo/Root Execution, Credential Access
  • Supply ChainUnpinned Dependencies, External Script Fetching, Obfuscated Code
Findings (6)

External Transmission

Medium
Category
Data Exfiltration
Content
.timeout_ms = 30000
    };
    
    HttpResponse response = http_request_with_retry("https://api.example.com/data", &config);
    
    if (response.status_code == 200) {
        printf("Success after %d attempts\n", response.attempt_count);
Confidence
50% confidence
Finding
Data is being sent to an external URL. This could be legitimate telemetry or data exfiltration. Manual review is recommended.

External Transmission

Medium
Category
Data Exfiltration
Content
.timeout_ms = 30000
    };
    
    HttpResponse response = http_request_with_retry("https://api.example.com/data", &config);
    
    if (response.status_code == 200) {
        printf("Success after %d attempts\n", response.attempt_count);
Confidence
50% confidence
Finding
Data is being sent to an external URL. This could be legitimate telemetry or data exfiltration. Manual review is recommended.

External Transmission

Medium
Category
Data Exfiltration
Content
.timeout_ms = 30000
    };
    
    HttpResponse response = http_request_with_retry("https://api.example.com/data", &config);
    
    if (response.status_code == 200) {
        printf("Success after %d attempts\n", response.attempt_count);
Confidence
50% confidence
Finding
Data is being sent to an external URL. This could be legitimate telemetry or data exfiltration. Manual review is recommended.

Intent-Code Divergence

Medium
Confidence
98% confidence
Finding
The core retry function claims to execute HTTP requests using the configured callback, but it never calls request_func and instead simulates unconditional success with status 200. This can make higher-level security logic believe network operations, health checks, authentication calls, or policy enforcement succeeded when nothing was actually performed.

Intent-Code Divergence

Medium
Confidence
97% confidence
Finding
The POST helper is documented and named as if it performs an HTTP POST, but it actually routes to the GET helper and ignores the supplied request body. This can cause security-sensitive callers to make the wrong HTTP method to remote endpoints, potentially leaking assumptions, bypassing intended server-side semantics, or causing unsafe behavior in integrations that rely on method correctness.

Missing User Warnings

Low
Confidence
91% confidence
Finding
This markdown file describes a universal HTTP retry mechanism and emphasizes automatic handling of API calls, but it does not disclose that using the skill may send user or system data over the network and may repeat those transmissions multiple times. For markdown files, omission of privacy or system-impact warnings for networked behavior is in scope for missing user warnings.

Static analysis

No suspicious patterns detected.