From 36908e8ef0d372a83e947ec2ed8141729453df27 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 4 Mar 2025 12:11:49 +0000 Subject: [PATCH 1/3] Do not track taint for keys in `sync.Map` There is no way to get the value of a key out of a `sync.Map`. --- go/ql/lib/ext/sync.model.yml | 8 ++-- .../go/frameworks/StdlibTaintFlow/Sync.go | 47 ------------------- 2 files changed, 4 insertions(+), 51 deletions(-) diff --git a/go/ql/lib/ext/sync.model.yml b/go/ql/lib/ext/sync.model.yml index da03ce0acc27..5a318552f531 100644 --- a/go/ql/lib/ext/sync.model.yml +++ b/go/ql/lib/ext/sync.model.yml @@ -6,10 +6,10 @@ extensions: - ["sync", "Map", True, "CompareAndSwap", "", "", "Argument[2]", "Argument[receiver]", "taint", "manual"] - ["sync", "Map", True, "Load", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"] - ["sync", "Map", True, "LoadOrStore", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"] - - ["sync", "Map", True, "LoadOrStore", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"] - - ["sync", "Map", True, "LoadOrStore", "", "", "Argument[0..1]", "ReturnValue[0]", "taint", "manual"] - - ["sync", "Map", True, "Store", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"] + - ["sync", "Map", True, "LoadOrStore", "", "", "Argument[1]", "Argument[receiver]", "taint", "manual"] + - ["sync", "Map", True, "LoadOrStore", "", "", "Argument[1]", "ReturnValue[0]", "taint", "manual"] + - ["sync", "Map", True, "Store", "", "", "Argument[1]", "Argument[receiver]", "taint", "manual"] - ["sync", "Map", True, "Swap", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"] - - ["sync", "Map", True, "Swap", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"] + - ["sync", "Map", True, "Swap", "", "", "Argument[1]", "Argument[receiver]", "taint", "manual"] - ["sync", "Pool", True, "Get", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"] - ["sync", "Pool", True, "Put", "", "", "Argument[0]", "Argument[receiver]", "taint", "manual"] diff --git a/go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Sync.go b/go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Sync.go index 25a0f7d96d02..d7d1044acd6f 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Sync.go +++ b/go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Sync.go @@ -16,20 +16,6 @@ func TaintStepTest_SyncMapLoadOrStore_B0I0O0(sourceCQL interface{}) interface{} return intoInterface650 } -func TaintStepTest_SyncMapLoadOrStore_B1I0O0(sourceCQL interface{}) interface{} { - fromInterface784 := sourceCQL.(interface{}) - var intoMap957 sync.Map - intoMap957.LoadOrStore(fromInterface784, nil) - return intoMap957 -} - -func TaintStepTest_SyncMapLoadOrStore_B1I0O1(sourceCQL interface{}) interface{} { - fromInterface520 := sourceCQL.(interface{}) - var mediumObjCQL sync.Map - intoInterface443, _ := mediumObjCQL.LoadOrStore(fromInterface520, nil) - return intoInterface443 -} - func TaintStepTest_SyncMapLoadOrStore_B1I1O0(sourceCQL interface{}) interface{} { fromInterface127 := sourceCQL.(interface{}) var intoMap483 sync.Map @@ -44,13 +30,6 @@ func TaintStepTest_SyncMapLoadOrStore_B1I1O1(sourceCQL interface{}) interface{} return intoInterface982 } -func TaintStepTest_SyncMapStore_B0I0O0(sourceCQL interface{}) interface{} { - fromInterface417 := sourceCQL.(interface{}) - var intoMap584 sync.Map - intoMap584.Store(fromInterface417, nil) - return intoMap584 -} - func TaintStepTest_SyncMapStore_B0I1O0(sourceCQL interface{}) interface{} { fromInterface991 := sourceCQL.(interface{}) var intoMap881 sync.Map @@ -58,12 +37,6 @@ func TaintStepTest_SyncMapStore_B0I1O0(sourceCQL interface{}) interface{} { return intoMap881 } -func TaintStepTest_SyncMapSwapinkey(sourceCQL interface{}) interface{} { - var m sync.Map - m.Swap(sourceCQL, "value") - return m -} - func TaintStepTest_SyncMapSwapinvalue(sourceCQL interface{}) interface{} { var m sync.Map m.Swap("key", sourceCQL) @@ -106,16 +79,6 @@ func RunAllTaints_Sync() { out := TaintStepTest_SyncMapLoadOrStore_B0I0O0(source) sink(1, out) } - { - source := newSource(2) - out := TaintStepTest_SyncMapLoadOrStore_B1I0O0(source) - sink(2, out) - } - { - source := newSource(3) - out := TaintStepTest_SyncMapLoadOrStore_B1I0O1(source) - sink(3, out) - } { source := newSource(4) out := TaintStepTest_SyncMapLoadOrStore_B1I1O0(source) @@ -126,11 +89,6 @@ func RunAllTaints_Sync() { out := TaintStepTest_SyncMapLoadOrStore_B1I1O1(source) sink(5, out) } - { - source := newSource(6) - out := TaintStepTest_SyncMapStore_B0I0O0(source) - sink(6, out) - } { source := newSource(7) out := TaintStepTest_SyncMapStore_B0I1O0(source) @@ -146,11 +104,6 @@ func RunAllTaints_Sync() { out := TaintStepTest_SyncPoolPut_B0I0O0(source) sink(9, out) } - { - source := newSource(10) - out := TaintStepTest_SyncMapSwapinkey(source) - sink(10, out) - } { source := newSource(11) out := TaintStepTest_SyncMapSwapinvalue(source) From 07c041483d806db62992fc934879b902d18af5d3 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 4 Mar 2025 12:14:43 +0000 Subject: [PATCH 2/3] Add change note --- .../change-notes/2025-03-04-improve-models-for-sync-map.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 go/ql/lib/change-notes/2025-03-04-improve-models-for-sync-map.md diff --git a/go/ql/lib/change-notes/2025-03-04-improve-models-for-sync-map.md b/go/ql/lib/change-notes/2025-03-04-improve-models-for-sync-map.md new file mode 100644 index 000000000000..b6ff3bf25b73 --- /dev/null +++ b/go/ql/lib/change-notes/2025-03-04-improve-models-for-sync-map.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* We no longer track taint into a `sync.Map` via the key of a key-value pair, since there is no way of reading the keys stored in the `sync.Map` back. From e2456ea59e6d91684111886ebdf42ed13592a3c2 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com> Date: Tue, 4 Mar 2025 15:07:24 +0000 Subject: [PATCH 3/3] Update go/ql/lib/change-notes/2025-03-04-improve-models-for-sync-map.md Co-authored-by: Michael B. Gale --- .../lib/change-notes/2025-03-04-improve-models-for-sync-map.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/ql/lib/change-notes/2025-03-04-improve-models-for-sync-map.md b/go/ql/lib/change-notes/2025-03-04-improve-models-for-sync-map.md index b6ff3bf25b73..ec0a167993cd 100644 --- a/go/ql/lib/change-notes/2025-03-04-improve-models-for-sync-map.md +++ b/go/ql/lib/change-notes/2025-03-04-improve-models-for-sync-map.md @@ -1,4 +1,4 @@ --- category: minorAnalysis --- -* We no longer track taint into a `sync.Map` via the key of a key-value pair, since there is no way of reading the keys stored in the `sync.Map` back. +* We no longer track taint into a `sync.Map` via the key of a key-value pair, since we do not model any way in which keys can be read from a `sync.Map`.