diff --git a/.github/workflows/swift.yml b/.github/workflows/swift.yml index 9e736e3225b..ad7e8f52aa3 100644 --- a/.github/workflows/swift.yml +++ b/.github/workflows/swift.yml @@ -68,21 +68,6 @@ jobs: steps: - uses: actions/checkout@v4 - uses: ./swift/actions/run-ql-tests - integration-tests-linux: - if: github.repository_owner == 'github' - needs: build-and-test-linux - runs-on: ubuntu-latest-xl - steps: - - uses: actions/checkout@v4 - - uses: ./swift/actions/run-integration-tests - integration-tests-macos: - if: ${{ github.repository_owner == 'github' && github.event_name == 'pull_request' }} - needs: build-and-test-macos - runs-on: macos-12-xl - timeout-minutes: 60 - steps: - - uses: actions/checkout@v4 - - uses: ./swift/actions/run-integration-tests clang-format: if : ${{ github.event_name == 'pull_request' }} runs-on: ubuntu-latest diff --git a/java/ql/lib/semmle/code/java/security/SpringCsrfProtection.qll b/java/ql/lib/semmle/code/java/security/SpringCsrfProtection.qll index cba95d4454d..c4259ee5b9d 100644 --- a/java/ql/lib/semmle/code/java/security/SpringCsrfProtection.qll +++ b/java/ql/lib/semmle/code/java/security/SpringCsrfProtection.qll @@ -17,4 +17,18 @@ predicate disablesSpringCsrfProtection(MethodCall call) { .getReferencedCallable() .hasQualifiedName("org.springframework.security.config.annotation.web.configurers", "AbstractHttpConfigurer", "disable") + or + call.getMethod().hasName("disable") and + call.getReceiverType() + .hasQualifiedName("org.springframework.security.config.web.server", + "ServerHttpSecurity$CsrfSpec") + or + call.getMethod() + .hasQualifiedName("org.springframework.security.config.web.server", "ServerHttpSecurity", + "csrf") and + call.getArgument(0) + .(MemberRefExpr) + .getReferencedCallable() + .hasQualifiedName("org.springframework.security.config.web.server", + "ServerHttpSecurity$CsrfSpec", "disable") } diff --git a/java/ql/src/change-notes/2024-05-30-disabled-csrf-query.md b/java/ql/src/change-notes/2024-05-30-disabled-csrf-query.md new file mode 100644 index 00000000000..2a2b5e33159 --- /dev/null +++ b/java/ql/src/change-notes/2024-05-30-disabled-csrf-query.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The query `java/spring-disabled-csrf-protection` detects disabling CSRF via `ServerHttpSecurity$CsrfSpec::disable`. \ No newline at end of file diff --git a/java/ql/test/query-tests/security/CWE-352/SpringCsrfProtectionTest.java b/java/ql/test/query-tests/security/CWE-352/SpringCsrfProtectionTest.java index 7e1e92e4392..a0a020193eb 100644 --- a/java/ql/test/query-tests/security/CWE-352/SpringCsrfProtectionTest.java +++ b/java/ql/test/query-tests/security/CWE-352/SpringCsrfProtectionTest.java @@ -1,10 +1,15 @@ import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; +import org.springframework.security.config.web.server.ServerHttpSecurity; public class SpringCsrfProtectionTest { - protected void test(HttpSecurity http) throws Exception { + protected void test(HttpSecurity http, final ServerHttpSecurity httpSecurity) throws Exception { http.csrf(csrf -> csrf.disable()); // $ hasSpringCsrfProtectionDisabled http.csrf().disable(); // $ hasSpringCsrfProtectionDisabled http.csrf(AbstractHttpConfigurer::disable); // $ hasSpringCsrfProtectionDisabled + + httpSecurity.csrf(csrf -> csrf.disable()); // $ hasSpringCsrfProtectionDisabled + httpSecurity.csrf().disable(); // $ hasSpringCsrfProtectionDisabled + httpSecurity.csrf(ServerHttpSecurity.CsrfSpec::disable); // $ hasSpringCsrfProtectionDisabled } -} +} \ No newline at end of file diff --git a/java/ql/test/stubs/springframework-5.3.8/org/springframework/security/config/web/server/ServerHttpSecurity.java b/java/ql/test/stubs/springframework-5.3.8/org/springframework/security/config/web/server/ServerHttpSecurity.java new file mode 100644 index 00000000000..5619a4bdfc9 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.3.8/org/springframework/security/config/web/server/ServerHttpSecurity.java @@ -0,0 +1,36 @@ +package org.springframework.security.config.web.server; + +import org.springframework.security.config.Customizer; + +public class ServerHttpSecurity { + private CsrfSpec csrf = new CsrfSpec(); + + protected ServerHttpSecurity() { + } + + public CsrfSpec csrf() { + if (this.csrf == null) { + this.csrf = new CsrfSpec(); + } + return this.csrf; + } + + public ServerHttpSecurity csrf(Customizer csrfCustomizer) { + if (this.csrf == null) { + this.csrf = new CsrfSpec(); + } + csrfCustomizer.customize(this.csrf); + return this; + } + + public final class CsrfSpec { + + private CsrfSpec() { + } + + public ServerHttpSecurity disable() { + ServerHttpSecurity.this.csrf = null; + return ServerHttpSecurity.this; + } + } +} diff --git a/misc/bazel/pkg.bzl b/misc/bazel/pkg.bzl index 8ba69677a1f..95bcd8e0bdb 100644 --- a/misc/bazel/pkg.bzl +++ b/misc/bazel/pkg.bzl @@ -72,6 +72,7 @@ def codeql_pkg_files( def _extract_pkg_filegroup_impl(ctx): src = ctx.attr.src[PackageFilegroupInfo] + arch_overrides = ctx.attr.arch_overrides platform = _detect_platform(ctx) if src.pkg_dirs or src.pkg_symlinks: @@ -82,8 +83,11 @@ def _extract_pkg_filegroup_impl(ctx): dest_src_map = {} for dest, file in pfi.dest_src_map.items(): file_kind, expanded_dest = _expand_path(dest, platform) + if file_kind == "generic" and dest in arch_overrides: + file_kind = "arch" if file_kind == ctx.attr.kind: dest_src_map[expanded_dest] = file + if dest_src_map: pkg_files.append((PackageFilesInfo(dest_src_map = dest_src_map, attributes = pfi.attributes), origin)) @@ -101,12 +105,14 @@ _extract_pkg_filegroup = rule( destination paths to the relevant codeql platform (linux64, win64 or osx64). The distinction between generic and arch contents is given on a per-file basis depending on the install path containing {CODEQL_PLATFORM}, which will typically have been added by a `prefix` attribute to a `pkg_*` rule. + Files that are arch-specific, but outside of the `CODEQL_PLATFORM` path can be specified in `arch_overrides`. No `pkg_dirs` or `pkg_symlink` must have been used for assembling the source mapping information: we could easily add support for that, but we don't require it for now. """, attrs = { "src": attr.label(providers = [PackageFilegroupInfo, DefaultInfo]), "kind": attr.string(doc = "What part to extract", values = ["generic", "arch"]), + "arch_overrides": attr.string_list(doc = "A list of files that should be included in the arch package regardless of the path"), } | OS_DETECTION_ATTRS, ) @@ -253,21 +259,26 @@ def codeql_pack( visibility = None, install_dest = "extractor-pack", compression_level = None, + arch_overrides = None, + zip_prefix = None, **kwargs): """ Define a codeql pack. This macro accepts `pkg_files`, `pkg_filegroup` or their `codeql_*` counterparts as `srcs`. - `zips` is a map from prefixes to `.zip` files to import. + `zips` is a map from `.zip` files to prefixes to import. * defines a `-generic-zip` target creating a `-generic.zip` archive with the generic bits, - prefixed with `name` + prefixed with `zip_prefix` * defines a `-arch-zip` target creating a `-.zip` archive with the - arch-specific bits, prefixed with `name` + arch-specific bits, prefixed with `zip_prefix` * defines a runnable `-installer` target that will install the pack in `install_dest`, relative to where the rule is used. The install destination can be overridden appending `-- --destdir=...` to the `bazel run` - invocation. This installation _does not_ prefix the contents with `name`. + invocation. This installation _does not_ prefix the contents with `zip_prefix`. + The prefix for the zip files can be set with `zip_prefix`, it is `name` by default. The distinction between arch-specific and generic contents is made based on whether the paths (including possible prefixes added by rules) contain the special `{CODEQL_PLATFORM}` placeholder, which in case it is present will also be replaced by the appropriate platform (`linux64`, `win64` or `osx64`). + Specific file paths can be placed in the arch-specific package by adding them to `arch_overrides`, even if their + path doesn't contain the `CODEQL_PLATFORM` placeholder. `compression_level` can be used to tweak the compression level used when creating archives. Consider that this does not affect the contents of `zips`, only `srcs`. @@ -275,6 +286,8 @@ def codeql_pack( internal = _make_internal(name) zip_filename = zip_filename or name zips = zips or {} + if zip_prefix == None: + zip_prefix = name pkg_filegroup( name = internal("all"), srcs = srcs, @@ -292,6 +305,7 @@ def codeql_pack( name = internal(kind), src = internal("all"), kind = kind, + arch_overrides = arch_overrides, visibility = ["//visibility:private"], ) if zips: @@ -311,7 +325,7 @@ def codeql_pack( name = internal(kind, "zip"), srcs = [internal(kind, "zip-base"), internal(kind, "zip-info")], out = _get_zip_filename(name, kind), - prefix = name, + prefix = zip_prefix, visibility = visibility, ) else: @@ -319,7 +333,7 @@ def codeql_pack( name = internal(kind, "zip"), srcs = [internal(kind)], visibility = visibility, - package_dir = name, + package_dir = zip_prefix, package_file_name = _get_zip_filename(name, kind), compression_level = compression_level, ) diff --git a/ql/extractor/src/extractor.rs b/ql/extractor/src/extractor.rs index 487f1de08a8..b50cae32a01 100644 --- a/ql/extractor/src/extractor.rs +++ b/ql/extractor/src/extractor.rs @@ -53,7 +53,7 @@ pub fn run(options: Options) -> std::io::Result<()> { trap_dir: options.output_dir, trap_compression: trap::Compression::from_env("CODEQL_QL_TRAP_COMPRESSION"), source_archive_dir: options.source_archive_dir, - file_list: options.file_list, + file_lists: vec![options.file_list], }; extractor.run() diff --git a/shared/tree-sitter-extractor/src/extractor/simple.rs b/shared/tree-sitter-extractor/src/extractor/simple.rs index baf620d19a3..89fa03288a7 100644 --- a/shared/tree-sitter-extractor/src/extractor/simple.rs +++ b/shared/tree-sitter-extractor/src/extractor/simple.rs @@ -20,7 +20,7 @@ pub struct Extractor { pub languages: Vec, pub trap_dir: PathBuf, pub source_archive_dir: PathBuf, - pub file_list: PathBuf, + pub file_lists: Vec, // Typically constructed via `trap::Compression::from_env`. // This allow us to report the error using our diagnostics system // without exposing it to consumers. @@ -74,7 +74,14 @@ impl Extractor { .build_global() .unwrap(); - let file_list = File::open(&self.file_list)?; + let file_lists: Vec = self + .file_lists + .iter() + .map(|file_list| { + File::open(file_list) + .unwrap_or_else(|_| panic!("Unable to open file list at {:?}", file_list)) + }) + .collect(); let mut schemas = vec![]; for lang in &self.languages { @@ -103,8 +110,10 @@ impl Extractor { ) }; - let lines: std::io::Result> = - std::io::BufReader::new(file_list).lines().collect(); + let lines: std::io::Result> = file_lists + .iter() + .flat_map(|file_list| std::io::BufReader::new(file_list).lines()) + .collect(); let lines = lines?; lines diff --git a/shared/tree-sitter-extractor/tests/integration_test.rs b/shared/tree-sitter-extractor/tests/integration_test.rs index a40f5a3f361..29058016764 100644 --- a/shared/tree-sitter-extractor/tests/integration_test.rs +++ b/shared/tree-sitter-extractor/tests/integration_test.rs @@ -30,7 +30,7 @@ fn simple_extractor() { languages: vec![language], trap_dir, source_archive_dir, - file_list, + file_lists: vec![file_list], trap_compression: Ok(trap::Compression::Gzip), }; diff --git a/shared/tree-sitter-extractor/tests/multiple_languages.rs b/shared/tree-sitter-extractor/tests/multiple_languages.rs index 483b90a2d7c..12a505f0170 100644 --- a/shared/tree-sitter-extractor/tests/multiple_languages.rs +++ b/shared/tree-sitter-extractor/tests/multiple_languages.rs @@ -39,7 +39,7 @@ fn multiple_language_extractor() { languages: vec![lang_ql, lang_json], trap_dir, source_archive_dir, - file_list, + file_lists: vec![file_list], trap_compression: Ok(trap::Compression::Gzip), }; diff --git a/swift/actions/run-integration-tests/action.yml b/swift/actions/run-integration-tests/action.yml deleted file mode 100644 index 0efd2371b96..00000000000 --- a/swift/actions/run-integration-tests/action.yml +++ /dev/null @@ -1,31 +0,0 @@ -name: Build Swift CodeQL pack -description: Builds the Swift CodeQL pack -runs: - using: composite - steps: - - uses: ./swift/actions/share-extractor-pack - - uses: actions/setup-python@v4 - with: - python-version-file: 'swift/.python-version' - - uses: redsun82/setup-swift@b2b6f77ab14f6a9b136b520dc53ec8eca27d2b99 - with: - swift-version: "5.8" - - uses: ./.github/actions/fetch-codeql - - id: query-cache - uses: ./.github/actions/cache-query-compilation - with: - key: swift-integration - - name: Run integration tests - shell: bash - run: | - python swift/ql/integration-tests/runner.py --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}" - env: - SEMMLE_DEBUG_TRACER: 10000 - - name: Upload test logs - if: ${{ always() }} - uses: actions/upload-artifact@v3 - with: - name: swift-integration-tests-logs-${{ runner.os }} - path: | - swift/ql/integration-tests/**/*db/log - retention-days: 1