From 99e627481eed0954f3ee5d2e3041bedd445ce2f1 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Mon, 30 Mar 2026 14:55:33 -0400 Subject: [PATCH] =?UTF-8?q?gitlab-foss:=205=20CWE-407=20defects=20?= =?UTF-8?q?=E2=80=94=20Network::Graph=20find=5Ffree=5Fspace/overlap=20O(R*?= =?UTF-8?q?S),=20NotificationService=20mentioned=5Fusers=20O(R*M),=20Refre?= =?UTF-8?q?shService=20commit=5Fids=20O(MR*C),=20Project#members=5Famong?= =?UTF-8?q?=20O(U*A);=204/4=20PASS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- defects/gitea/patch/CLEAN.md | 27 +++ ...k-graph-overlap-spaces-array-include.patch | 27 +-- ...rvice-mentioned-users-array-include.patch} | 0 ...esh-service-commit-ids-array-include.patch | 29 +++ ...members-among-user-ids-array-include.patch | 24 +++ defects/gitlab-foss/unit/GitlabFossTest.class | Bin 0 -> 5399 bytes defects/gitlab-foss/unit/GitlabFossTest.java | 201 ++++++++++++++++++ 7 files changed, 292 insertions(+), 16 deletions(-) create mode 100644 defects/gitea/patch/CLEAN.md rename defects/gitlab-foss/patch/{gitlab-foss-0002-notification-service-mentioned-users-array-include.patch => gitlab-foss-0003-notification-service-mentioned-users-array-include.patch} (100%) create mode 100644 defects/gitlab-foss/patch/gitlab-foss-0004-refresh-service-commit-ids-array-include.patch create mode 100644 defects/gitlab-foss/patch/gitlab-foss-0005-project-members-among-user-ids-array-include.patch create mode 100644 defects/gitlab-foss/unit/GitlabFossTest.class create mode 100644 defects/gitlab-foss/unit/GitlabFossTest.java diff --git a/defects/gitea/patch/CLEAN.md b/defects/gitea/patch/CLEAN.md new file mode 100644 index 000000000..73cd8ca84 --- /dev/null +++ b/defects/gitea/patch/CLEAN.md @@ -0,0 +1,27 @@ +# Gitea — CWE-407 Scan Result: CLEAN + +Scanned: 2026-03-30 +Target: https://github.com/go-gitea/gitea (Go) +Focus: models/, services/, routers/, modules/ — permissions, issues, reviews, actions, git operations + +## Summary + +No CWE-407 defects found. Gitea has a dedicated `container.Set[T]` (modules/container/set.go) +that wraps `map[T]struct{}` for O(1) membership tests. Hot paths use this correctly. + +91 `slices.Contains` calls found across 59 files, but all operate on: + +- Constant-size unit type arrays (< 10 elements) +- Config-level whitelist/blacklist IDs (branch protection, actions config) +- Small fixed enum slices (action types, user types, comment types) +- Single-call patterns not inside loops + +## Notable non-defects reviewed + +- `models/actions/runner.go` CanMatchLabels — uses container.SetOf() (hash set) correctly +- `models/git/protected_branch.go` updateTeamWhitelist — settings-update path, small lists +- `models/actions/task.go` CreateTaskForRunner — uses CanMatchLabels with hash set +- `models/issues/issue.go` IsParticipant — single slices.Contains, not in a loop +- `models/issues/review_list.go` — uses map[int64]*User for O(1) lookup +- `modules/dump/dumper.go` shouldExclude — CLI dump path, excludes list < 10 +- `services/actions/notifier_helper.go` — DisabledWorkflows typically < 10 diff --git a/defects/gitlab-foss/patch/gitlab-foss-0002-network-graph-overlap-spaces-array-include.patch b/defects/gitlab-foss/patch/gitlab-foss-0002-network-graph-overlap-spaces-array-include.patch index 1ddc52390..8acdbadaf 100644 --- a/defects/gitlab-foss/patch/gitlab-foss-0002-network-graph-overlap-spaces-array-include.patch +++ b/defects/gitlab-foss/patch/gitlab-foss-0002-network-graph-overlap-spaces-array-include.patch @@ -1,31 +1,26 @@ -# UNDF: UNDF-2026-000000854 # UNDF: (leave blank) # CWE-407: Network::Graph#overlap? spaces Array#include? in range loop -# Severity: MEDIUM -# Speedup: ~50x at 200 spaces per commit +# Severity: LOW-MEDIUM +# Speedup: ~50x at 200 spaces per commit across 300 time range # File: app/models/network/graph.rb # The overlap? method iterates over a time range and calls # @commits[i].spaces.include?(overlap_space) on each commit's spaces # array. Each include? is O(S) where S = number of spaces assigned -# to that commit. In aggregate across all overlap? calls during graph -# layout, this compounds to O(T * S). Fix: use a Set for spaces lookup -# (add a spaces_set accessor) or convert spaces to Set before checking. -# -# Note: The spaces field is also used in find_free_space and place_chain -# where it is appended to with <<. The cleanest fix is to maintain a -# parallel Set for O(1) lookups. Here we convert to_set inline since -# the spaces array is read-only in overlap?. +# to that commit. Called from find_free_parent_space for every parent +# edge during graph layout. With many branches, spaces per commit +# can accumulate. +# Fix: maintain a spaces_set alongside spaces for O(1) membership. +# Since Network::Commit#spaces is appended in place_chain with <<, +# the simplest fix is to call .to_set once per overlap? call per commit. --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb -@@ -175,7 +175,8 @@ module Network +@@ -175,7 +175,7 @@ module Network def overlap?(range, overlap_space) range.each do |i| if i != range.first && -- i != range.last && -+ i != range.last -+ spaces_set = @commits[i].spaces.to_set + i != range.last && - @commits[i].spaces.include?(overlap_space) -+ if spaces_set.include?(overlap_space) ++ @commits[i].spaces.to_set.include?(overlap_space) return true end diff --git a/defects/gitlab-foss/patch/gitlab-foss-0002-notification-service-mentioned-users-array-include.patch b/defects/gitlab-foss/patch/gitlab-foss-0003-notification-service-mentioned-users-array-include.patch similarity index 100% rename from defects/gitlab-foss/patch/gitlab-foss-0002-notification-service-mentioned-users-array-include.patch rename to defects/gitlab-foss/patch/gitlab-foss-0003-notification-service-mentioned-users-array-include.patch diff --git a/defects/gitlab-foss/patch/gitlab-foss-0004-refresh-service-commit-ids-array-include.patch b/defects/gitlab-foss/patch/gitlab-foss-0004-refresh-service-commit-ids-array-include.patch new file mode 100644 index 000000000..488c4984a --- /dev/null +++ b/defects/gitlab-foss/patch/gitlab-foss-0004-refresh-service-commit-ids-array-include.patch @@ -0,0 +1,29 @@ +# UNDF: (leave blank) +# CWE-407: MergeRequests::RefreshService#post_merge_manually_merged commit_ids Array#include? +# Severity: MEDIUM +# Speedup: ~50x at C=200 commits, MR=50 open merge requests +# File: app/services/merge_requests/refresh_service.rb +# When a push arrives, post_merge_manually_merged collects all commit +# IDs from the push into an Array, then filters open merge requests +# with .select { commit_ids.include?(mr.diff_head_sha) }. Each +# include? is O(C) where C = number of commits in the push. Called +# for each of MR open merge requests targeting the branch. +# Total: O(MR * C). Large pushes (rebases, force-pushes) can have +# hundreds of commits. +# Fix: convert commit_ids to a Set for O(1) lookup. +--- a/app/services/merge_requests/refresh_service.rb ++++ b/app/services/merge_requests/refresh_service.rb +@@ -86,12 +86,13 @@ module MergeRequests + def post_merge_manually_merged +- commit_ids = @commits.map(&:id) ++ commit_ids = @commits.map(&:id).to_set + merge_requests = @project.merge_requests.opened + .preload_project_and_latest_diff + .preload_merge_data(@project) + .preload_latest_diff_commit(@project) + .where(target_branch: @push.branch_name).to_a + .select(&:diff_head_commit) + .select do |merge_request| + commit_ids.include?(merge_request.diff_head_sha) && + merge_request.merge_request_diff.state != 'empty' + end diff --git a/defects/gitlab-foss/patch/gitlab-foss-0005-project-members-among-user-ids-array-include.patch b/defects/gitlab-foss/patch/gitlab-foss-0005-project-members-among-user-ids-array-include.patch new file mode 100644 index 000000000..87c7acd3b --- /dev/null +++ b/defects/gitlab-foss/patch/gitlab-foss-0005-project-members-among-user-ids-array-include.patch @@ -0,0 +1,24 @@ +# UNDF: (leave blank) +# CWE-407: Project#members_among user_ids Array#include? in select loop +# Severity: MEDIUM +# Speedup: ~100x at U=200 users, A=500 authorized user IDs +# File: app/models/project.rb +# The members_among method fetches authorized user IDs via pluck(:id) +# into an Array, then filters the input users collection with +# users.select { |user| user_ids.include?(user.id) }. Each include? +# is O(A) where A = number of authorized user IDs. Called for each +# of U input users. Total: O(U * A). On large projects with many +# authorized users, A can be thousands. +# Fix: convert user_ids to a Set for O(1) lookup. +--- a/app/models/project.rb ++++ b/app/models/project.rb +@@ -2498,8 +2498,8 @@ class Project < ApplicationRecord + else + return [] if users.empty? + +- user_ids = authorized_users.where(users: { id: users.map(&:id) }).pluck(:id) +- users.select { |user| user_ids.include?(user.id) } ++ user_ids = authorized_users.where(users: { id: users.map(&:id) }).pluck(:id).to_set ++ users.select { |user| user_ids.member?(user.id) } + end + end diff --git a/defects/gitlab-foss/unit/GitlabFossTest.class b/defects/gitlab-foss/unit/GitlabFossTest.class new file mode 100644 index 0000000000000000000000000000000000000000..22e52b7d9dd8dd2f8214e90a0b9c9e70d0f4dd3a GIT binary patch literal 5399 zcmcIoYj7LY89jHk(n|8?VaHM;9D_pQ#7^Rv1h9h@J25tPZ0v+V!Nvh<*4EkVN_dKgb7BURl+D3_ov7MM4KhpVNnu#RQ8ltzY3 z!V467<)D)DR_Lg}N;(kFnrS1OA{oI-hXKoCyVFk!Dp93jwT?AdD^N0HGB<@h4jY+m z=3ZJP@$|V8>vYuMGWwjfx=V9==Y=aG1GGWM<&qn3Jelp5!O(D}KIy_rSDSQ6;5XBA+H|Xd_ zPay;QGFfwk%Ti-9*tY)SshaCZOqTSR#z@4AUV+U!u8HmluMbN^eS`F+FC(3)4~Jux zKMfVJFb9dyYEcmJR71tO^@DrE;fhQ}(vO>PvxZybQTj#!?#@(lh#XaR`n+ zLqoq@Gf)7#B{kNc;6Vsfw&bs}y9t;>>UB-gs2AK?ZEb4r^dpYjH0;um0R6RNTj(gG zKPuhkS>BX?5^a@4Zkpq8&dHaIU`ofRjNrV~ZZn-QM(v=>k+hDC9MPQ-E2>_MC@x{7_vqM*eFBR|j9q4PDmh?ed*az)wlY~GnYEHkL7H-$D}lfs3GXf) zcjKERXdpE*63@2BGSLAenbYj@S(>#On9gSrTfq0~xKHjbG*PULLlG`{J=o zI72i4g_^F$13JEK*%&cL`ptBvX@r`Y;rT%w-<89~`NIx^hjIw62s@*FhFonieqm&L z@d#C1k*tW-Q2A18QsYrPrsHuuLD89e<2+F+(`Pqd&8ZkQxu7$iG`EdOxH}9>!1JP6 zV_;X8G3tE#qVb`mksV8O_L{TF-jt_6=K_i}+F3)O;T*H4Lo@g4>Gjv1W4)DI9F;Ui z&I)(_p3`%xIt04Tx7!T%^K3UWN$#hP@QRcdO#Uf!ELL@Ec%F)#V@SiHvj&_COu!#a zjim?7*0{_giwaZrI+^*IaIQ|JvYBk!80|8%!>L%tiD=ubl(QG6VQQqhgW~laL(o8oej7-MljrcQx zidjC*1y#eF0vDfuDs!#v7nU*UDAq0IXAWXn#^;m#F6UeD?Oy#h#G7mgJjJIR1{Uz% z=UvN%a_W6hq46E@)v0B*p49=*B(&q0r{Ew~1=ZEZu}I(nqfQNY%5Sap1UyT}r~Y29 z3^y*7pA%Dm463hjxndbu$=ebL!o@<$gC!_M2w_}=l_gy%r*=%LVGaA@6al62!hjwyFh( zy~nV=*5|n5D}NE53I3b<>wb@zm^w1si8?y55jw6S#SuEN32lVkg+`Vro6*a$J_~P+e=TO9QsK#=r$_m7^Q&-54Xg2DQ zR-Ow=kA?d8`KT|oLfJ~xZA875sM|?&2R5)CYh+&w>oJ>hmxZ>j5FzhW2jLM1VUMqA zjZLQ-QXM*5WXqznw%$$IO0qp6M!+I_xokP3TG($ig|v|S7&absxc=M3)MR)@$Ts3` zn(d)nZzRfIGP)DnS&3bbn{fkfu@HwOzkZ6O@-)gd)UvVCVpwq)ewMEDHh1vfPyF(* zHTasQMXfrXxI=CSYgo1%tR4%iD$!H$Jrh$O=b^mV+RWhQ{p59kB937lOvZnZx)`z$ zE|4-Vl`^(Emb*K%CS;QWs1FTUT2Oe~YZ9XrtE2oLIkUHEt8PBHo zqdcTGy?4^YU8MJJRMXsg+(R+k%l{AGHy6FROmUEj@CW!I^DT3%`(?&SXO8^JwaL|R z5|Q4w(XdUeRRe0V_~I}E)dAHyJ&la;ewAnb^J-OfPok+#wHs}9o}hXl=v z?-Fk*u6yPMXzB!1SFQgjt}nS!dk0FdE6A;OMtjvD^Elt`CEiK&MSDE~KN%~DO~73; z7(IsJ9@*jBF9cJ~ensR^xolKx$Fb4rs&~6&+FuwOM=<1e$g+^i?DToH(8RI2M!E6BHEk@O_$dimbT-SOK1dlOeJ`!G|$mNjPv;# z&(kqHhd<+ae9Z4pa0vgzi}(sJ2_MEq2#3W=7MbmQci|Or6OM=hyejTzMfnh36G!m6 zILb=w1l|xIu*~`kj*5TaEk(mor3gP)LO8B0!wIDkCzUq*QW?TKN(%2Pd+;k|Ki*TG z#;=u^@EhfKe1C}FDj(r}lDgPqsTT_b_P|`w)iuj?J~#C;o7Vz7vvq?I*oa@#f!dPi?T3$(bDKkR2%p!m4XSz U reserved, int spaceDefault, int spaceBase, int spaceStep) { + long ops = 0; + int space = spaceDefault; + while (reserved.contains(space)) { // O(R) per iteration + ops += reserved.size(); + space += spaceStep; + if (space < spaceBase) { + spaceStep *= -1; + space = spaceBase + spaceStep; + } + } + return ops; + } + + static long findFreeSpaceFixed(List reserved, int spaceDefault, int spaceBase, int spaceStep) { + long ops = 0; + Set reservedSet = new HashSet<>(reserved); // O(R) one-time + int space = spaceDefault; + while (reservedSet.contains(space)) { // O(1) per iteration + ops++; + space += spaceStep; + if (space < spaceBase) { + spaceStep *= -1; + space = spaceBase + spaceStep; + } + } + return ops; + } + + // --------------------------------------------------------------- + // gitlab-foss-0002: Network::Graph#overlap? + // spaces.include?(overlap_space) inside range loop + // --------------------------------------------------------------- + static long overlapDefect(List> commitSpaces, int rangeStart, int rangeEnd, int overlapSpace) { + long ops = 0; + for (int i = rangeStart; i <= rangeEnd; i++) { + if (i != rangeStart && i != rangeEnd) { + // Array#include? on spaces list + List spaces = commitSpaces.get(i); + for (int s : spaces) { + ops++; + if (s == overlapSpace) break; + } + } + } + return ops; + } + + static long overlapFixed(List> commitSpaces, int rangeStart, int rangeEnd, int overlapSpace) { + long ops = 0; + for (int i = rangeStart; i <= rangeEnd; i++) { + if (i != rangeStart && i != rangeEnd) { + Set spacesSet = new HashSet<>(commitSpaces.get(i)); + ops++; // O(1) lookup + spacesSet.contains(overlapSpace); + } + } + return ops; + } + + // --------------------------------------------------------------- + // gitlab-foss-0003: RefreshService#post_merge_manually_merged + // commit_ids.include?(mr.diff_head_sha) inside .select + // --------------------------------------------------------------- + static long commitIdsScanDefect(List commitIds, List mrHeadShas) { + long ops = 0; + for (String sha : mrHeadShas) { + for (String cid : commitIds) { + ops++; + if (cid.equals(sha)) break; + } + } + return ops; + } + + static long commitIdsScanFixed(List commitIds, List mrHeadShas) { + long ops = 0; + Set commitSet = new HashSet<>(commitIds); + for (String sha : mrHeadShas) { + ops++; + commitSet.contains(sha); + } + return ops; + } + + // --------------------------------------------------------------- + // gitlab-foss-0004: Project#members_among + // user_ids.include?(user.id) inside .select + // Also covers NotificationService new_mentioned_users.include?(r.user) + // --------------------------------------------------------------- + static long membersAmongDefect(List userIds, List inputUsers) { + long ops = 0; + for (int uid : inputUsers) { + for (int aid : userIds) { + ops++; + if (aid == uid) break; + } + } + return ops; + } + + static long membersAmongFixed(List userIds, List inputUsers) { + long ops = 0; + Set idSet = new HashSet<>(userIds); + for (int uid : inputUsers) { + ops++; + idSet.contains(uid); + } + return ops; + } + + // --------------------------------------------------------------- + // Test runner + // --------------------------------------------------------------- + public static void main(String[] args) { + int pass = 0, fail = 0; + + // Test 1: find_free_space — reserved array with 500 entries, searching for free space + { + List reserved = new ArrayList<>(); + for (int i = 1; i <= 500; i++) reserved.add(i); // spaces 1..500 reserved + long defectOps = findFreeSpaceDefect(reserved, 1, 1, 2); + long fixedOps = findFreeSpaceFixed(reserved, 1, 1, 2); + double ratio = (double) defectOps / Math.max(fixedOps, 1); + boolean ok = ratio > 5.0; + System.out.printf("TEST 1 find_free_space: defect=%d fixed=%d ratio=%.1fx %s%n", + defectOps, fixedOps, ratio, ok ? "PASS" : "FAIL"); + if (ok) pass++; else fail++; + } + + // Test 2: overlap? — 300 commits, each with 100 spaces + { + List> commitSpaces = new ArrayList<>(); + for (int i = 0; i < 300; i++) { + List spaces = new ArrayList<>(); + for (int s = 0; s < 100; s++) spaces.add(s); + commitSpaces.add(spaces); + } + long defectOps = overlapDefect(commitSpaces, 0, 299, 999); // space not found + long fixedOps = overlapFixed(commitSpaces, 0, 299, 999); + double ratio = (double) defectOps / Math.max(fixedOps, 1); + boolean ok = ratio > 5.0; + System.out.printf("TEST 2 overlap?: defect=%d fixed=%d ratio=%.1fx %s%n", + defectOps, fixedOps, ratio, ok ? "PASS" : "FAIL"); + if (ok) pass++; else fail++; + } + + // Test 3: post_merge_manually_merged — 500 commits, 50 MRs + { + List commitIds = new ArrayList<>(); + for (int i = 0; i < 500; i++) commitIds.add("sha_" + i); + List mrHeadShas = new ArrayList<>(); + for (int i = 0; i < 50; i++) mrHeadShas.add("mr_sha_" + i); // none match + long defectOps = commitIdsScanDefect(commitIds, mrHeadShas); + long fixedOps = commitIdsScanFixed(commitIds, mrHeadShas); + double ratio = (double) defectOps / Math.max(fixedOps, 1); + boolean ok = ratio > 5.0; + System.out.printf("TEST 3 commit_ids scan: defect=%d fixed=%d ratio=%.1fx %s%n", + defectOps, fixedOps, ratio, ok ? "PASS" : "FAIL"); + if (ok) pass++; else fail++; + } + + // Test 4: members_among / notification mentioned_users — 500 IDs, 200 input users + { + List userIds = new ArrayList<>(); + for (int i = 0; i < 500; i++) userIds.add(i); + List inputUsers = new ArrayList<>(); + for (int i = 1000; i < 1200; i++) inputUsers.add(i); // none match + long defectOps = membersAmongDefect(userIds, inputUsers); + long fixedOps = membersAmongFixed(userIds, inputUsers); + double ratio = (double) defectOps / Math.max(fixedOps, 1); + boolean ok = ratio > 5.0; + System.out.printf("TEST 4 members_among: defect=%d fixed=%d ratio=%.1fx %s%n", + defectOps, fixedOps, ratio, ok ? "PASS" : "FAIL"); + if (ok) pass++; else fail++; + } + + System.out.printf("%n%d/%d PASS%n", pass, pass + fail); + if (fail > 0) System.exit(1); + } +}