Skip to content

Commit 1623bba

Browse files
MathiasVPMathiasVP
committed
Merge branch 'main' into no-dtt-in-tainted-arithmetic
2 parents c950e26 + ae09499 commit 1623bba

File tree

21 files changed

+385
-148
lines changed

21 files changed

+385
-148
lines changed

‎cpp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,9 @@ IEnumerable<string> IBuildActions.EnumerateDirectories(string dir)
145145

146146
bool IBuildActions.IsMacOs() => IsMacOs;
147147

148-
public bool IsArm { get; set; }
148+
public bool IsRunningOnAppleSilicon { get; set; }
149149

150-
bool IBuildActions.IsArm() => IsArm;
150+
bool IBuildActions.IsRunningOnAppleSilicon() => IsRunningOnAppleSilicon;
151151

152152
string IBuildActions.PathCombine(params string[] parts)
153153
{

‎cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,24 +52,25 @@ predicate isUnboundedWrite(BufferWrite bw) {
5252
* Holds if `e` is a source buffer going into an unbounded write `bw` or a
5353
* qualifier of (a qualifier of ...) such a source.
5454
*/
55-
predicate unboundedWriteSource(Expr e, BufferWrite bw) {
56-
isUnboundedWrite(bw) and e = bw.getASource()
55+
predicate unboundedWriteSource(Expr e, BufferWrite bw, boolean qualifier) {
56+
isUnboundedWrite(bw) and e = bw.getASource() and qualifier = false
5757
or
58-
exists(FieldAccess fa | unboundedWriteSource(fa, bw) and e = fa.getQualifier())
58+
exists(FieldAccess fa | unboundedWriteSource(fa, bw, _) and e = fa.getQualifier()) and
59+
qualifier = true
5960
}
6061

6162
predicate isSource(FS::FlowSource source, string sourceType) { source.getSourceType() = sourceType }
6263

63-
predicate isSink(DataFlow::Node sink, BufferWrite bw) {
64-
unboundedWriteSource(sink.asIndirectExpr(), bw)
64+
predicate isSink(DataFlow::Node sink, BufferWrite bw, boolean qualifier) {
65+
unboundedWriteSource(sink.asIndirectExpr(), bw, qualifier)
6566
or
6667
// `gets` and `scanf` reads from stdin so there's no real input.
6768
// The `BufferWrite` library models this as the call itself being
6869
// the source. In this case we mark the output argument as being
6970
// the sink so that we report a path where source = sink (because
7071
// the same output argument is also included in `isSource`).
7172
bw.getASource() = bw and
72-
unboundedWriteSource(sink.asDefiningArgument(), bw)
73+
unboundedWriteSource(sink.asDefiningArgument(), bw, qualifier)
7374
}
7475

7576
predicate lessThanOrEqual(IRGuardCondition g, Expr e, boolean branch) {
@@ -84,9 +85,9 @@ predicate lessThanOrEqual(IRGuardCondition g, Expr e, boolean branch) {
8485
module Config implements DataFlow::ConfigSig {
8586
predicate isSource(DataFlow::Node source) { isSource(source, _) }
8687

87-
predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
88+
predicate isSink(DataFlow::Node sink) { isSink(sink, _, _) }
8889

89-
predicate isBarrierOut(DataFlow::Node node) { isSink(node) }
90+
predicate isBarrierOut(DataFlow::Node node) { isSink(node, _, false) }
9091

9192
predicate isBarrier(DataFlow::Node node) {
9293
// Block flow if the node is guarded by any <, <= or = operations.
@@ -116,7 +117,7 @@ from BufferWrite bw, Flow::PathNode source, Flow::PathNode sink, string sourceTy
116117
where
117118
Flow::flowPath(source, sink) and
118119
isSource(source.getNode(), sourceType) and
119-
isSink(sink.getNode(), bw)
120+
isSink(sink.getNode(), bw, _)
120121
select bw, source, sink,
121122
"This '" + bw.getBWDesc() + "' with input from $@ may overflow the destination.",
122123
source.getNode(), sourceType

‎cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/UnboundedWrite.expected

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,32 @@
11
edges
22
| main.cpp:6:27:6:30 | argv indirection | main.cpp:10:20:10:23 | argv indirection |
3-
| main.cpp:10:20:10:23 | argv indirection | tests.cpp:618:32:618:35 | argv indirection |
3+
| main.cpp:10:20:10:23 | argv indirection | tests.cpp:631:32:631:35 | argv indirection |
44
| tests.cpp:613:19:613:24 | source indirection | tests.cpp:615:17:615:22 | source indirection |
5-
| tests.cpp:618:32:618:35 | argv indirection | tests.cpp:643:9:643:15 | access to array indirection |
6-
| tests.cpp:643:9:643:15 | access to array indirection | tests.cpp:613:19:613:24 | source indirection |
5+
| tests.cpp:622:19:622:24 | source indirection | tests.cpp:625:2:625:16 | ... = ... indirection |
6+
| tests.cpp:625:2:625:16 | ... = ... indirection | tests.cpp:625:4:625:7 | s indirection [post update] [home indirection] |
7+
| tests.cpp:625:4:625:7 | s indirection [post update] [home indirection] | tests.cpp:628:14:628:14 | s indirection [home indirection] |
8+
| tests.cpp:628:14:628:14 | s indirection [home indirection] | tests.cpp:628:14:628:19 | home indirection |
9+
| tests.cpp:628:14:628:14 | s indirection [home indirection] | tests.cpp:628:16:628:19 | home indirection |
10+
| tests.cpp:628:16:628:19 | home indirection | tests.cpp:628:14:628:19 | home indirection |
11+
| tests.cpp:631:32:631:35 | argv indirection | tests.cpp:656:9:656:15 | access to array indirection |
12+
| tests.cpp:631:32:631:35 | argv indirection | tests.cpp:657:9:657:15 | access to array indirection |
13+
| tests.cpp:656:9:656:15 | access to array indirection | tests.cpp:613:19:613:24 | source indirection |
14+
| tests.cpp:657:9:657:15 | access to array indirection | tests.cpp:622:19:622:24 | source indirection |
715
nodes
816
| main.cpp:6:27:6:30 | argv indirection | semmle.label | argv indirection |
917
| main.cpp:10:20:10:23 | argv indirection | semmle.label | argv indirection |
1018
| tests.cpp:613:19:613:24 | source indirection | semmle.label | source indirection |
1119
| tests.cpp:615:17:615:22 | source indirection | semmle.label | source indirection |
12-
| tests.cpp:618:32:618:35 | argv indirection | semmle.label | argv indirection |
13-
| tests.cpp:643:9:643:15 | access to array indirection | semmle.label | access to array indirection |
20+
| tests.cpp:622:19:622:24 | source indirection | semmle.label | source indirection |
21+
| tests.cpp:625:2:625:16 | ... = ... indirection | semmle.label | ... = ... indirection |
22+
| tests.cpp:625:4:625:7 | s indirection [post update] [home indirection] | semmle.label | s indirection [post update] [home indirection] |
23+
| tests.cpp:628:14:628:14 | s indirection [home indirection] | semmle.label | s indirection [home indirection] |
24+
| tests.cpp:628:14:628:19 | home indirection | semmle.label | home indirection |
25+
| tests.cpp:628:16:628:19 | home indirection | semmle.label | home indirection |
26+
| tests.cpp:631:32:631:35 | argv indirection | semmle.label | argv indirection |
27+
| tests.cpp:656:9:656:15 | access to array indirection | semmle.label | access to array indirection |
28+
| tests.cpp:657:9:657:15 | access to array indirection | semmle.label | access to array indirection |
1429
subpaths
1530
#select
1631
| tests.cpp:615:2:615:7 | call to strcpy | main.cpp:6:27:6:30 | argv indirection | tests.cpp:615:17:615:22 | source indirection | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | argv indirection | a command-line argument |
32+
| tests.cpp:628:2:628:7 | call to strcpy | main.cpp:6:27:6:30 | argv indirection | tests.cpp:628:14:628:19 | home indirection | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | argv indirection | a command-line argument |

‎cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/tests.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,19 @@ void test24(char* source) {
615615
strcpy(buffer, source); // BAD
616616
}
617617

618+
struct my_struct {
619+
char* home;
620+
};
621+
622+
void test25(char* source) {
623+
my_struct s;
624+
625+
s.home = source;
626+
627+
char buf[100];
628+
strcpy(buf, s.home); // BAD
629+
}
630+
618631
int tests_main(int argc, char *argv[])
619632
{
620633
long long arr17[19];
@@ -641,6 +654,7 @@ int tests_main(int argc, char *argv[])
641654
test22(argc == 0, argv[0]);
642655
test23();
643656
test24(argv[0]);
657+
test25(argv[0]);
644658

645659
return 0;
646660
}

‎csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,9 @@ IEnumerable<string> IBuildActions.EnumerateDirectories(string dir)
159159

160160
bool IBuildActions.IsMacOs() => IsMacOs;
161161

162-
public bool IsArm { get; set; }
162+
public bool IsRunningOnAppleSilicon { get; set; }
163163

164-
bool IBuildActions.IsArm() => IsArm;
164+
bool IBuildActions.IsRunningOnAppleSilicon() => IsRunningOnAppleSilicon;
165165

166166
public string PathCombine(params string[] parts)
167167
{

‎csharp/autobuilder/Semmle.Autobuild.Shared/BuildActions.cs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Diagnostics;
44
using System.Diagnostics.CodeAnalysis;
55
using System.IO;
6+
using System.Linq;
67
using System.Runtime.InteropServices;
78
using System.Xml;
89
using Semmle.Util;
@@ -119,10 +120,10 @@ public interface IBuildActions
119120
bool IsMacOs();
120121

121122
/// <summary>
122-
/// Gets a value indicating whether we are running on arm.
123+
/// Gets a value indicating whether we are running on Apple Silicon.
123124
/// </summary>
124-
/// <returns>True if we are running on arm.</returns>
125-
bool IsArm();
125+
/// <returns>True if we are running on Apple Silicon.</returns>
126+
bool IsRunningOnAppleSilicon();
126127

127128
/// <summary>
128129
/// Combine path segments, Path.Combine().
@@ -240,9 +241,25 @@ int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory,
240241

241242
bool IBuildActions.IsMacOs() => RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
242243

243-
bool IBuildActions.IsArm() =>
244-
RuntimeInformation.ProcessArchitecture == Architecture.Arm64 ||
245-
RuntimeInformation.ProcessArchitecture == Architecture.Arm;
244+
bool IBuildActions.IsRunningOnAppleSilicon()
245+
{
246+
var thisBuildActions = (IBuildActions)this;
247+
248+
if (!thisBuildActions.IsMacOs())
249+
{
250+
return false;
251+
}
252+
253+
try
254+
{
255+
thisBuildActions.RunProcess("sysctl", "machdep.cpu.brand_string", workingDirectory: null, env: null, out var stdOut);
256+
return stdOut?.Any(s => s?.ToLowerInvariant().Contains("apple") == true) ?? false;
257+
}
258+
catch (Exception)
259+
{
260+
return false;
261+
}
262+
}
246263

247264
string IBuildActions.PathCombine(params string[] parts) => Path.Combine(parts);
248265

‎csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,12 @@ internal static class MsBuildCommandExtensions
1515
/// <returns></returns>
1616
public static CommandBuilder MsBuildCommand(this CommandBuilder cmdBuilder, IAutobuilder<AutobuildOptionsShared> builder)
1717
{
18-
var isArmMac = builder.Actions.IsMacOs() && builder.Actions.IsArm();
19-
2018
// mono doesn't ship with `msbuild` on Arm-based Macs, but we can fall back to
2119
// msbuild that ships with `dotnet` which can be invoked with `dotnet msbuild`
2220
// perhaps we should do this on all platforms?
23-
return isArmMac ?
24-
cmdBuilder.RunCommand("dotnet").Argument("msbuild") :
25-
cmdBuilder.RunCommand("msbuild");
21+
return builder.Actions.IsRunningOnAppleSilicon()
22+
? cmdBuilder.RunCommand("dotnet").Argument("msbuild")
23+
: cmdBuilder.RunCommand("msbuild");
2624
}
2725
}
2826

‎csharp/ql/integration-tests/posix-only/standalone_dependencies_nuget/skip-on-platform-osx-arm

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Skipping the test on the ARM runners, as we're running into trouble with Mono and nuget.

‎ql/Cargo.lock

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ql/extractor/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ tree-sitter-blame = {path = "../buramu/tree-sitter-blame"}
1414
tree-sitter-json = {git = "https://github.com/tausbn/tree-sitter-json.git", rev = "745663ee997f1576fe1e7187e6347e0db36ec7a9"}
1515
clap = { version = "4.2", features = ["derive"] }
1616
tracing = "0.1"
17-
tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
17+
tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
1818
rayon = "1.8.0"
1919
regex = "1.10.2"
2020
codeql-extractor = { path = "../../shared/tree-sitter-extractor" }

0 commit comments

Comments
 (0)