Potential fix for code scanning alert no. 5: Size computation for allocation may overflow - #9
Conversation
…ocation may overflow Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a security vulnerability (code scanning alert #5) related to potential integer overflow in buffer size allocation within the ChangeJsonKeys function. The fix adds a guard check to reject JSON inputs larger than 64MB before performing the allocation that adds 8 bytes to the input length.
Key Changes:
- Added size validation to prevent allocation overflow in
ChangeJsonKeys - Input exceeding 64MB threshold returns the original untransformed data
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Guard against allocation overflow: reject very large input | ||
| if len(input) > 64*1024*1024 { // 64MB | ||
| return input // Or nil, or error handling as you prefer | ||
| } |
There was a problem hiding this comment.
Returning the untransformed input when the size limit is exceeded could lead to incorrect behavior. The function is expected to transform JSON keys (e.g., camel case to snake case), but returning the original input would skip this transformation silently.
This could cause:
- API responses with inconsistent key naming (some camelCase, some snake_case)
- Consumers expecting transformed keys to receive untransformed data
- Silent data corruption where callers won't know the transformation failed
Consider one of these alternatives:
- Return an error (though this would require changing the function signature)
- Use a panic with a descriptive message for such exceptional cases
- Log a warning and continue with the existing error handling pattern (line 44 also returns input on error)
Given that line 44 already returns the untransformed input on JSON parsing errors, option 3 (adding logging) might be most consistent with the existing error handling approach.
|
|
||
| // Guard against allocation overflow: reject very large input | ||
| if len(input) > 64*1024*1024 { // 64MB | ||
| return input // Or nil, or error handling as you prefer |
There was a problem hiding this comment.
The inline comment "// Or nil, or error handling as you prefer" should be removed. This comment appears to be placeholder text from development and doesn't add value to production code. Comments should explain "why" not list alternative implementations.
| return input // Or nil, or error handling as you prefer | |
| return input |
Potential fix for https://github.com/LukeLarge/opentonapi/security/code-scanning/5
To fix this problem, we need to ensure that the computation of the buffer size in
ChangeJsonKeys(input []byte, f func(s string) string)does not overflow. The best way is to check thatlen(input)is not excessively large before performing the addition for allocation. A reasonable upper limit for JSON sizes (as in the example: 64MB) ensures the buffer size always fits withinintwith ample margin for+8. Ifinputis larger than this threshold, return an error or the original input to avoid attempting a dangerous allocation.Change to make:
internal/g/camel_snake.go, edit the implementation ofChangeJsonKeys:if len(input) > 64*1024*1024(64 MB).Needed imports/methods/definitions:
Suggested fixes powered by Copilot Autofix. Review carefully before merging.