Skip to content

Commit 28fe94a

Browse files
pwdelgithub-advanced-security[bot]astrosnat
authored
Potential fix for code scanning alert no. 2: Incorrect conversion between integer types (#485)
* Potential fix for code scanning alert no. 2: Incorrect conversion between integer types Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Updating having eliminated magic numbers --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Osnat Katz Moon <137817983+astrosnat@users.noreply.github.com>
1 parent c87c267 commit 28fe94a

File tree

2 files changed

+85
-1
lines changed

2 files changed

+85
-1
lines changed

README/README-CONVENTIONS.md

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,68 @@ db := util.GetDB()
143143

144144
- When there is no meaningful dependency that needs to be passed.
145145
- When it makes the code unnecessarily complex.
146-
- When simple handler functions would be more readable and sufficient.
146+
- When simple handler functions would be more readable and sufficient.
147+
148+
### 32-Bit Platform Compatibility
149+
150+
**Convention UUID: CONV-32BIT-001**
151+
152+
* When parsing string values to uint64 and then converting to platform-specific uint types, always validate that the value fits within the platform's uint size to prevent overflow on 32-bit systems.
153+
* This convention addresses security concerns identified by GitHub's CodeQL analysis and ensures cross-platform compatibility.
154+
155+
#### The Problem
156+
157+
* On 64-bit platforms, `uint` is 64 bits and can hold any `uint64` value safely.
158+
* On 32-bit platforms, `uint` is only 32 bits, so large `uint64` values will overflow when converted to `uint`.
159+
* Direct conversion without validation can lead to unexpected behavior and potential security issues.
160+
161+
#### Recommended Implementation
162+
163+
Use named constants to make the platform detection logic clear and self-documenting:
164+
165+
```go
166+
// Platform detection constants for 32-bit compatibility check
167+
const (
168+
bitsInByte = 8
169+
bytesInUint32 = 4
170+
rightShiftFor64BitDetection = 63
171+
baseBitWidth = 32
172+
)
173+
174+
// Detect platform bit width using named constants
175+
maxUintValue := ^uint(0)
176+
platformBitWidth := baseBitWidth << (maxUintValue >> rightShiftFor64BitDetection)
177+
isPlatform32Bit := platformBitWidth == baseBitWidth
178+
179+
// Validate that the uint64 value fits in platform uint
180+
if isPlatform32Bit && valueUint64 > math.MaxUint32 {
181+
http.Error(w, "Value out of range for platform", http.StatusBadRequest)
182+
return
183+
}
184+
valueUint := uint(valueUint64)
185+
```
186+
187+
#### What NOT To Do (Avoid Magic Numbers)
188+
189+
```go
190+
// BAD: Uses magic numbers without explanation
191+
if uintSize := 32 << (^uint(0) >> 63); uintSize == 32 && valueUint64 > math.MaxUint32 {
192+
// This is unclear and unmaintainable
193+
}
194+
```
195+
196+
#### How The Platform Detection Works
197+
198+
* `^uint(0)` creates the maximum value for the platform's uint type (all bits set to 1)
199+
* On 64-bit platforms: `^uint(0)` has 64 ones, so `>> 63` yields 1, making `32 << 1 = 64`
200+
* On 32-bit platforms: `^uint(0)` has 32 ones, so `>> 63` yields 0, making `32 << 0 = 32`
201+
* This allows runtime detection of whether we're on a 32-bit or 64-bit platform
202+
203+
#### Usage Pattern
204+
205+
Apply this pattern whenever:
206+
- Parsing string IDs to uint64 and then converting to uint
207+
- Handling user input that gets converted to platform-specific types
208+
- Working with numeric values that might exceed 32-bit limits
209+
210+
To find all implementations of this convention, search for "Convention CONV-32BIT-001" in the codebase.

backend/handlers/markets/marketdetailshandler.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package marketshandlers
22

33
import (
44
"encoding/json"
5+
"math"
56
"net/http"
67
"socialpredict/handlers/marketpublicresponse"
78
marketmath "socialpredict/handlers/math/market"
@@ -36,6 +37,25 @@ func MarketDetailsHandler(w http.ResponseWriter, r *http.Request) {
3637
return
3738
}
3839

40+
// 32-bit platform compatibility check (Convention CONV-32BIT-001 in README-CONVENTIONS.md)
41+
// Platform detection constants for 32-bit compatibility check
42+
const (
43+
bitsInByte = 8
44+
bytesInUint32 = 4
45+
rightShiftFor64BitDetection = 63
46+
baseBitWidth = 32
47+
)
48+
49+
// Detect platform bit width using named constants
50+
maxUintValue := ^uint(0)
51+
platformBitWidth := baseBitWidth << (maxUintValue >> rightShiftFor64BitDetection)
52+
isPlatform32Bit := platformBitWidth == baseBitWidth
53+
54+
// Validate that the uint64 value fits in platform uint
55+
if isPlatform32Bit && marketIDUint64 > math.MaxUint32 {
56+
http.Error(w, "Market ID out of range", http.StatusBadRequest)
57+
return
58+
}
3959
marketIDUint := uint(marketIDUint64)
4060

4161
// open up database to utilize connection pooling

0 commit comments

Comments
 (0)