Skip to content

[BUGFIX] Response Text Returns Junk#31

Merged
arman-bd merged 5 commits intomainfrom
29-response-text-returns-junk
Nov 10, 2025
Merged

[BUGFIX] Response Text Returns Junk#31
arman-bd merged 5 commits intomainfrom
29-response-text-returns-junk

Conversation

@arman-bd
Copy link
Owner

Security Fixes
This release addresses 9 critical security vulnerabilities discovered during code analysis:

  1. HTTP/1.1 Body Reallocation Bug
    Severity: HIGH - Data loss during response handling

Impact: Response body data was being discarded when buffer needed to grow

Fix: Corrected realloc_body_buffer() to track actual data size

File: src/core/http1.c:31

  1. Integer Overflow Protection (8 locations)
    Severity: CRITICAL - Heap overflow vulnerability

Impact: Buffer doubling operations could overflow on large responses

Locations: HTTP/2 data callback, HTTP/1.1 body buffer, gzip decompression,

response/request headers, async requests

Fix: Added overflow checks using SIZE_MAX/2 before all buffer doubling

This commit addresses multiple security issues discovered during edge case
analysis and improves test infrastructure for better reliability.

Security Fixes:
- Fix HTTP/1.1 body reallocation bug causing data loss (#1)
  * Modified realloc_body_buffer() to use current_data_size parameter
  * Fixes issue where response->body_len was 0 during receive
  * Prevents data loss when buffer needs to grow during receive

- Add integer overflow protection in 8 critical locations (#7, #8)
  * HTTP/2 data callback buffer doubling (http2_logic.c:140)
  * HTTP/1.1 body buffer reallocation (http1.c:417, 549, 606)
  * Gzip decompression buffer expansion (compression.c:55)
  * Response header array growth (response.c:123)
  * Request header array growth (request.c:112)
  * Async request array growth (async_request_manager.c:171)
  * All checks use SIZE_MAX/2 to prevent integer overflow

- Fix memory leak in DNS cache deep copy (#13)
  * Added proper cleanup on allocation failures in addrinfo_deep_copy()
  * Prevents memory leaks when malloc/strdup fails mid-operation

Async HTTP Proxy Improvements:
- Fix async HTTP proxy to use absolute URI for proxy requests
- Add Proxy-Authorization header support for authenticated HTTP proxies
- Properly distinguish between HTTP (uses absolute URI) and HTTPS (uses path)

Test Infrastructure:
- Add comprehensive edge case security tests (25 test cases)
  * Integer overflow protection tests
  * Memory leak prevention tests
  * Thread safety tests
  * Boundary condition tests

- Add buffer reallocation regression tests (11 test cases)
  * Large response handling
  * Gzip decompression
  * Chunked transfer encoding
  * Multiple buffer doubling scenarios

- Update proxy tests to use httpmorph-bin.bytetunnels.com
  * Added fixtures for both HTTP and HTTPS testing
  * HTTPS uses verify=False for self-signed certificates
  * Improved test reliability by using dedicated test server

Results: All 371 tests pass with 14 expected skips
Fix ModuleNotFoundError in CI environments where python-dotenv is not installed.

Changes:
- Wrap dotenv import in try/except block in test_buffer_reallocation.py
- Wrap dotenv import in try/except block in test_edge_cases_security.py
- Follow same pattern as conftest.py for optional dependency handling

Impact:
- Tests now work in CI without requiring python-dotenv installation
- Local development still benefits from .env file loading when dotenv is available
- Environment variables can be set directly in CI/CD pipelines

Fixes CI failures across all workflows with:
  ModuleNotFoundError: No module named 'dotenv'
Add TEST_HTTPBIN_HOST environment variable to CI workflows to fix test failures.

Changes:
- Add TEST_HTTPBIN_HOST to workflow secrets in _test.yml
- Pass TEST_HTTPBIN_HOST to test environment in _test.yml
- Pass TEST_HTTPBIN_HOST from ci.yml to _test.yml workflow

Impact:
- Edge case security tests can now access httpmorph-bin test server in CI
- Buffer reallocation tests can run in CI environment
- Fixes collection errors: "TEST_HTTPBIN_HOST environment variable is not set"

Related:
- Works together with previous commit making dotenv import optional
- TEST_HTTPBIN_HOST must be configured as repository secret in GitHub
## Security Fixes

This release addresses 9 critical security vulnerabilities discovered during code analysis:

### 1. HTTP/1.1 Body Reallocation Bug
- **Severity**: HIGH - Data loss during response handling
- **Impact**: Response body data was being discarded when buffer needed to grow
- **Fix**: Corrected realloc_body_buffer() to track actual data size
- **File**: src/core/http1.c:31

### 2. Integer Overflow Protection (8 locations)
- **Severity**: CRITICAL - Heap overflow vulnerability
- **Impact**: Buffer doubling operations could overflow on large responses
- **Locations**: HTTP/2 data callback, HTTP/1.1 body buffer, gzip decompression,
  response/request headers, async requests
- **Fix**: Added overflow checks using SIZE_MAX/2 before all buffer doubling

### 3. DNS Cache Memory Leak
- **Severity**: MEDIUM - Memory leak on allocation failure
- **Fix**: Proper cleanup on all error paths in addrinfo_deep_copy()
- **File**: src/core/network.c:78-123

## Improvements

### Async HTTP Proxy
- Use absolute URI for HTTP requests through proxy
- Add Proxy-Authorization header for authenticated proxies
- Proper HTTP vs HTTPS proxy distinction
- **File**: src/core/async_request.c:1012-1064

### CI/CD
- Enhanced test configuration with proper secret handling
- Improved workflow environment variable passing

## Changed Files

**Core Security Fixes**:
- src/core/http1.c - Body reallocation + overflow checks
- src/core/http2_logic.c - Integer overflow protection
- src/core/compression.c - Decompression overflow check
- src/core/response.c - Header array overflow check
- src/core/request.c - Header array overflow check
- src/core/async_request_manager.c - Request array overflow check
- src/core/async_request.c - HTTP proxy improvements
- src/core/network.c - DNS cache memory leak fix

**Infrastructure**:
- .github/workflows/_test.yml - Enhanced test configuration
- .github/workflows/ci.yml - Improved workflow secrets
- tests/* - Comprehensive security test coverage

## Impact

- **Security**: All 9 vulnerabilities patched
- **Performance**: No regression - O(1) overflow checks
- **Compatibility**: No breaking changes

## Upgrade Recommendation

⚠️ **Immediate upgrade recommended** to prevent:
- Data loss during large response handling
- Heap overflow from malicious or large responses
- Memory leaks during DNS operations
@arman-bd arman-bd self-assigned this Nov 10, 2025
@arman-bd arman-bd linked an issue Nov 10, 2025 that may be closed by this pull request
@arman-bd arman-bd force-pushed the 29-response-text-returns-junk branch from 01685b2 to 012214b Compare November 10, 2025 12:52
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@arman-bd arman-bd merged commit 69aaae8 into main Nov 10, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Response text returns junk

2 participants