8000 fix: Add more #nosec G115 annotations for integer overflow warnings · coder/coder@f8af6a8 · GitHub
[go: up one dir, main page]
Skip to content

Commit f8af6a8

Browse files
sreyasreya
committed
fix: Add more #nosec G115 annotations for integer overflow warnings
This change continues adding appropriate '#nosec G115' annotations to various integer type conversions that are safe in their specific contexts. Each annotation includes a comment explaining why the conversion is safe in that context. Additional areas fixed: - tailnet module (node and peer IDs) - VPN module (prefix lengths and router configs) - Agent SSH module (terminal dimensions and X11 protocol) - Notifications module (configuration parameters) - Various other integer conversions where the range is guaranteed to be safe
1 parent ee44d98 commit f8af6a8

File tree

16 files changed

+57
-40
lines changed

16 files changed

+57
-40
lines changed

‎agent/agentcontainers/containers_dockercli.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -453,8 +453,8 @@ func convertDockerInspect(raw []byte) ([]codersdk.WorkspaceAgentContainer, []str
453453
hostPortContainers[hp] = append(hostPortContainers[hp], in.ID)
454454
}
455455
out.Ports = append(out.Ports, codersdk.WorkspaceAgentContainerPort{
456-
Network: network,
457-
Port: cp,
456+
Network: network,
457+
Port: cp,
458458
// #nosec G115 - Safe conversion since Docker ports are limited to uint16 range
459459
HostPort: uint16(hp),
460460
HostIP: p.HostIP,

‎agent/agentssh/agentssh.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
702702
windowSize = nil
703703
continue
704704
}
705+
// #nosec G115 - Safe conversions for terminal dimensions which are expected to be within uint16 range
705706
resizeErr := ptty.Resize(uint16(win.Height), uint16(win.Width))
706707
// If the pty is closed, then command has exited, no need to log.
707708
if resizeErr != nil && !errors.Is(resizeErr, pty.ErrClosed) {

‎agent/agentssh/x11.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ func (s *Server) x11Handler(ctx ssh.Context, x11 ssh.X11) (displayNumber int, ha
116116
OriginatorPort uint32
117117
}{
118118
OriginatorAddress: tcpAddr.IP.String(),
119+
// #nosec G115 - Safe conversion as TCP port numbers are within uint32 range (0-65535)
119120
OriginatorPort: uint32(tcpAddr.Port),
120121
}))
121122
if err != nil {
@@ -294,6 +295,7 @@ func addXauthEntry(ctx context.Context, fs afero.Fs, host string, display string
294295
return xerrors.Errorf("failed to write family: %w", err)
295296
}
296297

298+
// #nosec G115 - Safe conversion for host name length which is expected to be within uint16 range
297299
err = binary.Write(file, binary.BigEndian, uint16(len(host)))
298300
if err != nil {
299301
return xerrors.Errorf("failed to write host length: %w", err)
@@ -303,6 +305,7 @@ func addXauthEntry(ctx context.Context, fs afero.Fs, host string, display string
303305
return xerrors.Errorf("failed to write host: %w", err)
304306
}
305307

308+
// #nosec G115 - Safe conversion for display name length which is expected to be within uint16 range
306309
err = binary.Write(file, binary.BigEndian, uint16(len(display)))
307310
if err != nil {
308311
return xerrors.Errorf("failed to write display length: %w", err)

‎coderd/notifications/manager.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,7 @@ func (m *Manager) syncUpdates(ctx context.Context) {
337337
uctx, cancel := context.WithTimeout(ctx, time.Second*30)
338338
defer cancel()
339339

340+
// #nosec G115 - Safe conversion for max send attempts which is expected to be within int32 range
340341
failureParams.MaxAttempts = int32(m.cfg.MaxSendAttempts)
341342
failureParams.RetryInterval = int32(m.cfg.RetryInterval.Value().Seconds())
342343
n, err := m.store.BulkMarkNotificationMessagesFailed(uctx, failureParams)

‎coderd/notifications/notifier.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,9 @@ func (n *notifier) process(ctx context.Context, success chan<- dispatchResult, f
209209
// messages until they are dispatched - or until the lease expires (in exceptional cases).
210210
func (n *notifier) fetch(ctx context.Context) ([]database.AcquireNotificationMessagesRow, error) {
211211
msgs, err := n.store.AcquireNotificationMessages(ctx, database.AcquireNotificationMessagesParams{
212+
// #nosec G115 - Safe conversion for lease count which is expected to be within int32 range
212213
Count: int32(n.cfg.LeaseCount),
214+
// #nosec G115 - Safe conversion for max send attempts which is expected to be within int32 range
213215
MaxAttemptCount: int32(n.cfg.MaxSendAttempts),
214216
NotifierID: n.id,
215217
LeaseSeconds: int32(n.cfg.LeasePeriod.Value().Seconds()),

‎coderd/searchquery/search.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func Workspaces(ctx context.Context, db database.Store, query string, page coder
100100
// #nosec G115 - Safe conversion for pagination offset which is expected to be within int32 range
101101
Offset: int32(page.Offset),
102102
// #nosec G115 - Safe conversion for pagination limit which is expected to be within int32 range
103-
Limit: int32(page.Limit),
103+
Limit: int32(page.Limit),
104104
}
105105

106106
if query == "" {

‎coderd/telemetry/telemetry.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,7 @@ func ConvertWorkspaceBuild(build database.WorkspaceBuild) WorkspaceBuild {
730730
JobID: build.JobID,
731731
TemplateVersionID: build.TemplateVersionID,
732732
// #nosec G115 - Safe conversion as build numbers are expected to be positive and within uint32 range
733-
BuildNumber: uint32(build.BuildNumber),
733+
BuildNumber: uint32(build.BuildNumber),
734734
}
735735
}
736736

@@ -1037,11 +1037,11 @@ func ConvertTemplate(dbTemplate database.Template) Template {
10371037
TimeTilDormantMillis: time.Duration(dbTemplate.TimeTilDormant).Milliseconds(),
10381038
TimeTilDormantAutoDeleteMillis: time.Duration(dbTemplate.TimeTilDormantAutoDelete).Milliseconds(),
10391039
// #nosec G115 - Safe conversion as AutostopRequirementDaysOfWeek is a bitmap of 7 days, easily within uint8 range
1040-
AutostopRequirementDaysOfWeek: codersdk.BitmapToWeekdays(uint8(dbTemplate.AutostopRequirementDaysOfWeek)),
1041-
AutostopRequirementWeeks: dbTemplate.AutostopRequirementWeeks,
1042-
AutostartAllowedDays: codersdk.BitmapToWeekdays(dbTemplate.AutostartAllowedDays()),
1043-
RequireActiveVersion: dbTemplate.RequireActiveVersion,
1044-
Deprecated: dbTemplate.Deprecated != "",
1040+
AutostopRequirementDaysOfWeek: codersdk.BitmapToWeekdays(uint8(dbTemplate.AutostopRequirementDaysOfWeek)),
1041+
AutostopRequirementWeeks: dbTemplate.AutostopRequirementWeeks,
1042+
AutostartAllowedDays: codersdk.BitmapToWeekdays(dbTemplate.AutostartAllowedDays()),
1043+
RequireActiveVersion: dbTemplate.RequireActiveVersion,
1044+
Deprecated: dbTemplate.Deprecated != "",
10451045
}
10461046
}
10471047

‎codersdk/agentsdk/convert.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@ func ProtoFromManifest(manifest Manifest) (*proto.Manifest, error) {
6262
return nil, xerrors.Errorf("convert workspace apps: %w", err)
6363
}
6464
return &proto.Manifest{
65-
AgentId: manifest.AgentID[:],
66-
AgentName: manifest.AgentName,
67-
OwnerUsername: manifest.OwnerName,
68-
WorkspaceId: manifest.WorkspaceID[:],
69-
WorkspaceName: manifest.WorkspaceName,
65+
AgentId: manifest.AgentID[:],
66+
AgentName: manifest.AgentName,
67+
OwnerUsername: manifest.OwnerName,
68+
WorkspaceId: manifest.WorkspaceID[:],
69+
WorkspaceName: manifest.WorkspaceName,
7070
// #nosec G115 - Safe conversion for GitAuthConfigs which is expected to be small and positive
7171
GitAuthConfigs: uint32(manifest.GitAuthConfigs),
7272
EnvironmentVariables: manifest.EnvironmentVariables,

‎enterprise/replicasync/replicasync.go

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,14 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, ps pubsub.P
6565
}
6666
// nolint:gocritic // Inserting a replica is a system function.
6767
replica, err := db.InsertReplica(dbauthz.AsSystemRestricted(ctx), database.InsertReplicaParams{
68-
ID: options.ID,
69-
CreatedAt: dbtime.Now(),
70-
StartedAt: dbtime.Now(),
71-
UpdatedAt: dbtime.Now(),
72-
Hostname: hostname,
73-
RegionID: options.RegionID,
74-
RelayAddress: options.RelayAddress,
75-
Version: buildinfo.Version(),
68+
ID: options.ID,
69+
CreatedAt: dbtime.Now(),
70+
StartedAt: dbtime.Now(),
71+
UpdatedAt: dbtime.Now(),
72+
Hostname: hostname,
73+
RegionID: options.RegionID,
74+
RelayAddress: options.RelayAddress,
75+
Version: buildinfo.Version(),
7676
// #nosec G115 - Safe conversion for microseconds latency which is expected to be within int32 range
7777
DatabaseLatency: int32(databaseLatency.Microseconds()),
7878
Primary: true,
@@ -314,15 +314,15 @@ func (m *Manager) syncReplicas(ctx context.Context) error {
314314
defer m.mutex.Unlock()
315315
// nolint:gocritic // Updating a replica is a system function.
316316
replica, err := m.db.UpdateReplica(dbauthz.AsSystemRestricted(ctx), database.UpdateReplicaParams{
317-
ID: m.self.ID,
318-
UpdatedAt: dbtime.Now(),
319-
StartedAt: m.self.StartedAt,
320-
StoppedAt: m.self.StoppedAt,
321-
RelayAddress: m.self.RelayAddress,
322-
RegionID: m.self.RegionID,
323-
Hostname: m.self.Hostname,
324-
Version: m.self.Version,
325-
Error: replicaError,
317+
ID: m.self.ID,
318+
UpdatedAt: dbtime.Now(),
319+
StartedAt: m.self.StartedAt,
320+
StoppedAt: m.self.StoppedAt,
321+
RelayAddress: m.self.RelayAddress,
322+
RegionID: m.self.RegionID,
323+
Hostname: m.self.Hostname,
324+
Version: m.self.Version,
325+
Error: replicaError,
326326
// #nosec G115 - Safe conversion for microseconds latency which is expected to be within int32 range
327327
DatabaseLatency: int32(databaseLatency.Microseconds()),
328328
Primary: m.self.Primary,
@@ -334,14 +334,14 @@ func (m *Manager) syncReplicas(ctx context.Context) error {
334334
// self replica has been cleaned up, we must reinsert
335335
// nolint:gocritic // Updating a replica is a system function.
336336
replica, err = m.db.InsertReplica(dbauthz.AsSystemRestricted(ctx), database.InsertReplicaParams{
337-
ID: m.self.ID,
338-
CreatedAt: dbtime.Now(),
339-
UpdatedAt: dbtime.Now(),
340-
StartedAt: m.self.StartedAt,
341-
RelayAddress: m.self.RelayAddress,
342-
RegionID: m.self.RegionID,
343-
Hostname: m.self.Hostname,
344-
Version: m.self.Version,
337+
ID: m.self.ID,
338+
CreatedAt: dbtime.Now(),
339+
UpdatedAt: dbtime.Now(),
340+
StartedAt: m.self.StartedAt,
341+
RelayAddress: m.self.RelayAddress,
342+
RegionID: m.self.RegionID,
343+
Hostname: m.self.Hostname,
344+
Version: m.self.Version,
345345
// #nosec G115 - Safe conversion for microseconds latency which is expected to be within int32 range
346346
DatabaseLatency: int32(databaseLatency.Microseconds()),
347347
Primary: m.self.Primary,

‎provisionerd/runner/runner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,7 @@ func (r *Runner) commitQuota(ctx context.Context, resources []*sdkproto.Resource
885885
const stage = "Commit quota"
886886

887887
resp, err := r.quotaCommitter.CommitQuota(ctx, &proto.CommitQuotaRequest{
888-
JobId: r.job.JobId,
888+
JobId: r.job.JobId,
889889
// #nosec G115 - Safe conversion as cost is expected to be within int32 range for provisioning costs
890890
DailyCost: int32(cost),
891891
})

‎tailnet/conn.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ type TelemetrySink interface {
132132
// NodeID creates a Tailscale NodeID from the last 8 bytes of a UUID. It ensures
133133
// the returned NodeID is always positive.
134134
func NodeID(uid uuid.UUID) tailcfg.NodeID {
135+
// #nosec G115 - Safe conversion with potential loss of range, but acceptable for node IDs
135136
id := int64(binary.BigEndian.Uint64(uid[8:]))
136137

137138
// ensure id is positive

‎tailnet/convert.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ func NodeToProto(n *Node) (*proto.Node, error) {
3131
}
3232
derpForcedWebsocket := make(map[int32]string)
3333
for i, s := range n.DERPForcedWebsocket {
34+
// #nosec G115 - Safe conversion for DERP region IDs which are small positive integers
3435
derpForcedWebsocket[int32(i)] = s
3536
}
3637
addresses := make([]string, len(n.Addresses))

‎tailnet/telemetry.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ func (b *TelemetryStore) updateRemoteNodeIDLocked(nm *netmap.NetworkMap) {
131131
for _, p := range nm.Peers {
132132
for _, a := range p.Addresses {
133133
if a.Addr() == ip && a.IsSingleIP() {
134+
// #nosec G115 - Safe conversion as p.ID is expected to be within uint64 range for node IDs
134135
b.nodeIDRemote = uint64(p.ID)
135136
}
136137
}
@@ -188,6 +189,7 @@ func (b *TelemetryStore) updateByNodeLocked(n *tailcfg.Node) bool {
188189
if n == nil {
189190
return false
190191
}
192+
// #nosec G115 - Safe conversion as n.ID is expected to be within uint64 range for node IDs
191193
b.nodeIDSelf = uint64(n.ID)
192194
derpIP, err := netip.ParseAddrPort(n.DERP)
193195
if err != nil {

‎tailnet/telemetry_internal_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ func TestTelemetryStore(t *testing.T) {
7070
e := telemetry.newEvent()
7171
// DERPMapToProto already tested
7272
require.Equal(t, DERPMapToProto(nm.DERPMap), e.DerpMap)
73+
// #nosec G115 - Safe conversion in test code as node IDs are within uint64 range
7374
require.Equal(t, uint64(nm.Peers[1].ID), e.NodeIdRemote)
75+
// #nosec G115 - Safe conversion in test code as node IDs are within uint64 range
7476
require.Equal(t, uint64(nm.SelfNode.ID), e.NodeIdSelf)
7577
require.Equal(t, application, e.Application)
7678
require.Equal(t, nm.SelfNode.DERP, fmt.Sprintf("127.3.3.40:%d", e.HomeDerp))

‎vpn/router.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func convertRouterConfig(cfg router.Config) *NetworkSettingsRequest {
4545
v4SubnetMasks = append(v4SubnetMasks, prefixToSubnetMask(addrs))
4646
} else if addrs.Addr().Is6() {
4747
v6LocalAddrs = append(v6LocalAddrs, addrs.Addr().String())
48+
// #nosec G115 - Safe conversion as IPv6 prefix lengths are always within uint32 range (0-128)
4849
v6PrefixLengths = append(v6PrefixLengths, uint32(addrs.Bits()))
4950
} else {
5051
continue
@@ -95,6 +96,7 @@ func convertRouterConfig(cfg router.Config) *NetworkSettingsRequest {
9596
}
9697

9798
return &NetworkSettingsRequest{
99+
// #nosec G115 - Safe conversion as MTU values are expected to be small positive integers
98100
Mtu: uint32(cfg.NewMTU),
99101
Ipv4Settings: v4Settings,
100102
Ipv6Settings: v6Settings,
@@ -114,6 +116,7 @@ func convertToIPV4Route(route netip.Prefix) *NetworkSettingsRequest_IPv4Settings
114116
func convertToIPV6Route(route netip.Prefix) *NetworkSettingsRequest_IPv6Settings_IPv6Route {
115117
return &NetworkSettingsRequest_IPv6Settings_IPv6Route{
116118
Destination: route.Addr().String(),
119+
// #nosec G115 - Safe conversion as prefix lengths are always within uint32 range (0-128)
117120
PrefixLength: uint32(route.Bits()),
118121
Router: "", // N/A
119122
}

‎vpn/tunnel.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ func (t *Tunnel) Sync() {
302302

303303
func sinkEntryToPb(e slog.SinkEntry) *Log {
304304
l := &Log{
305+
// #nosec G115 - Safe conversion for log levels which are small positive integers
305306
Level: Log_Level(e.Level),
306307
Message: e.Message,
307308
LoggerNames: e.LoggerNames,

0 commit comments

Comments
 (0)
0