Skip to content

fix(adhoc-sweep-fixes): CU-86akhf8u2 56 review findings across 40 files - #65

Draft
flamingo[bot] wants to merge 40 commits into
masterfrom
ai-fix/adhoc-sweep-fixes-ee91f228-f72ecbf3
Draft

flamingo[bot] wants to merge 40 commits into
masterfrom
ai-fix/adhoc-sweep-fixes-ee91f228-f72ecbf3

Conversation

@flamingo

@flamingo flamingo Bot commented Sep 14, 2026

Copy link
Copy Markdown

Closes 56 review findings across 40 files.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

# Fix confidence Finding Location
1 🟡 80 medium SQL injection via unsanitized string concatenation of key/domain into SQLite queries plugins/database/sqlite.cpp:118
2 🟡 75 medium scan() builds SQL query by concatenating unsanitized prefix parameter plugins/database/sqlite.cpp:275
3 🔴 45 low — review closely tryVacuum ignores sqlite3_exec status silently instead of returning Status plugins/database/sqlite.cpp:150
4 🟡 85 medium sqlite3_stmt leaked when sqlite3_step fails in put/remove/removeRange paths plugins/database/sqlite.cpp:214
5 🟢 95 high Python 2-only except syntax breaks this test file under Python 3 tools/tests/test_windows_service.py:106
6 🟡 85 medium Writing str content to files opened in binary mode ('wb') will raise TypeError in Python 3 tools/tests/test_windows_service.py:249
7 🟢 95 high Python 2 idiom x is not '' used for string comparison (identity vs equality bug) tools/tests/test_windows_service.py:110
8 🟢 92 high getProductName dereferences find() result without checking for end() — UB on unknown enum value osquery/tables/applications/jetbrains_plugins.cpp:28
9 🟢 90 high VLOG/LOG stream macros incorrectly suffixed with std::endl osquery/tables/applications/jetbrains_plugins.cpp:52
10 🔴 25 low — review closely archive_read_free failure result unused after logging osquery/tables/applications/jetbrains_plugins.cpp:38
11 🟢 95 high getProcessProperties dereferences getpwuid() result without checking for nullptr osquery/events/darwin/es_utils.cpp:159
12 🟢 95 high ec->cwd is computed twice in getProcessProperties osquery/events/darwin/es_utils.cpp:145
13 🟢 92 high WmiResultItem::GetLong / GetUnsignedLong / GetLongLong / GetUnsignedLongLong all read value.lVal instead of the correct union member osquery/core/windows/wmi.cpp:229
14 🟢 90 high GetUnsignedShort and GetUnsignedInt32 read value.uiVal even though VT_UI2/VT_UINT map to different VARIANT union members osquery/core/windows/wmi.cpp:195
15 🟢 90 high getFormFactor/getMemoryType compare signed 'id' against unsigned kFormFactors.size() causing wrong bounds check for negative WMI values osquery/tables/system/windows/smbios_tables.cpp:31
16 🟢 90 high getMemoryType has the same signed/unsigned comparison bug as getFormFactor osquery/tables/system/windows/smbios_tables.cpp:47
17 🟡 80 medium SCNetworkReachabilityContext stores address of a local shared_ptr reference, leading to dangling pointer osquery/events/darwin/scnetwork.cpp:46
18 🟢 90 high SCNetworkEventPublisher::configure() adds a target only when it ALREADY exists in target list, inverting intent osquery/events/darwin/scnetwork.cpp:114
19 🔴 55 low — review closely genWindowsSearch builds SQL/query filter strings directly from unsanitized user-supplied constraints (query, sort, additional_properties) osquery/tables/system/windows/windows_search.cpp:397
20 🟡 85 medium generateSqlFromUserQuery swallows COM failures by returning empty string instead of propagating Status osquery/tables/system/windows/windows_search.cpp:252
21 🟢 98 high pidsFromContext copies procs onto itself instead of appending the per-pid selection results osquery/tables/system/system_utils.cpp:34
22 🟢 97 high genPortageKeywordSummary skips parsing when unmasked content is non-empty due to inverted condition osquery/tables/system/linux/portage.cpp:317
23 🟢 97 high quicklook_cache.cpp does not check sqlite3_prepare_v2 return code before stepping, and finalizes a possibly-null stmt osquery/tables/system/darwin/quicklook_cache.cpp:120
24 🟢 97 high compileSingleFile is misplaced inside the anonymous namespace closing brace comment but declared as a public API in the header, and its closing brace mistakenly says '} // namespace osquery' osquery/tables/yara/yara_utils.cpp:196
25 🟡 85 medium dns_cache.cpp: use-after-free / double free of pEntry, and hLib handle never freed osquery/tables/system/windows/dns_cache.cpp:136
26 🟡 85 medium taskName is dereferenced via SysStringLen even when get_Name fails and BSTR is uninitialized osquery/tables/system/windows/scheduled_tasks.cpp:89
27 🟢 95 high writeFile in pidfile_posix.cpp discards computed remaining_bytes with a redundant self-overwriting statement osquery/utils/pidfile/pidfile_posix.cpp:114
28 🟢 92 high FSEvents configure() skips subscriptions whose discovered_ path is already non-empty, inverting the intended condition osquery/events/darwin/fsevents.cpp:231
29 🟡 70 medium enumFiles misclassifies symlinks as non-directories without following them osquery/events/linux/bpf/filesystem.cpp:128
30 🟡 75 medium populateSubkeys() default-parameter mismatch: called with 1 arg but declared to require replaceKeys default only via prototype not shown here osquery/tables/system/windows/registry.cpp:473
31 🟢 92 high compress()/decompress() leak the ZSTD stream context on early return via std::vector<void*> misuse osquery/filesystem/file_compression.cpp:51
32 🟢 92 high decompress() also uses std::vector<void*> instead of a byte buffer, same defect as compress() osquery/filesystem/file_compression.cpp:117
33 🟡 75 medium Carver::postCarve silently continues after failed block upload, still marks carve as SUCCESS osquery/carver/carver.cpp:352
34 🟡 70 medium blockwiseCopy silently drops write failures without reporting number of bytes actually written vs read osquery/carver/carver.cpp:271
35 🟢 90 high removeRange() indexes db_[domain] with operator[], creating an empty domain entry as a side effect of a range-delete on a non-existent domain osquery/database/ephemeral.cpp:168
36 🟢 90 high remove() uses operator[] on db_, silently creating an empty domain map when removing a key from a nonexistent domain osquery/database/ephemeral.cpp:155
37 🟢 92 high exclude_paths_.find() result never checked against end(), truthiness of iterator is undefined behavior osquery/events/linux/inotify.cpp:349
38 🔴 55 low — review closely malloc'd scratch buffer freed with free() but allocated via malloc — mismatched with C++ ownership idioms, and no RAII wrapper leaves leak risk on early-return paths osquery/events/linux/inotify.cpp:69
39 🔴 40 low — review closely Missing EXPECT_EQ(got, expected) assertion in all_drives_removed test osquery/tables/system/tests/linux/md_tables_tests.cpp:174
40 🟢 97 high Dead/unreachable Status::failure call in queryRpmDb — missing return osquery/tables/system/tests/linux/rpm_packages_tests.cpp:96
41 🟢 95 high EtwProviderConfig::isValid() checks getPostProcessor() twice instead of also validating getPreProcessor() osquery/events/windows/etw/etw_provider_config.cpp:25
42 🟢 97 high GetDismPackageFeatureStateName / state variable used uninitialized when GetUnsignedInt32 and GetLong both fail osquery/tables/system/windows/windows_optional_features.cpp:42
43 🟢 95 high genControlConfigFromPath dereferences line[0] without checking for empty line after trim osquery/tables/system/posix/system_controls.cpp:65
44 🟢 92 high getDeviceInformationSet ignores its guid_filter parameter and always uses HECI_INTERFACE_GUID osquery/tables/system/windows/intel_me.cpp:270
45 🟢 92 high Redundant re-declaration of selected_gids shadows outer variable in genGroups osquery/tables/system/windows/groups.cpp:67
46 🟢 95 high gen_api function output_path uses Python 2 style .split("\n") on subprocess bytes output without decoding tools/codegen/genapi.py:277
47 🟢 95 high getKeychainPath treats a failed SecKeychainGetPath call as success when path_size/keychain_path happen to be non-zero from stack garbage osquery/tables/system/darwin/keychain_utils.cpp:79
48 🟢 97 high cpu_info.cpp genCpuInfo pushes the same Row object repeatedly without resetting between WMI entries osquery/tables/system/windows/cpu_info.cpp:21
49 🟡 85 medium set_intersection requires sorted ranges but table_set/event_tables ordering is not guaranteed to match osquery/events/events.cpp:42
50 🟡 85 medium DynamicTableRow::get_column looks up column twice, second lookup skips missing-key branch inconsistently osquery/sql/dynamic_table_row.cpp:103
51 🟢 95 high getDellLegacyBiosInfo indexes vPossibleValuesDescription without checking its size matches vPossibleValues osquery/tables/system/windows/wmi_bios_info.cpp:126
52 🟡 72 medium KernelEtwSessionRunnable::start() races with pause()/resume() using unprotected traceSessionStopped_ busy-wait alongside a locked condition wait osquery/events/windows/etw/etw_kernel_session.cpp:135
53 🔴 55 low — review closely parseAttributes/getTagContent use ad-hoc regex XML parsing, not a real XML parser, for AppxManifest.xml osquery/tables/system/windows/programs.cpp:23
54 🟡 85 medium Pidfile::lockFile on Windows silently drops the write-mode upgrade on failure without matching the POSIX chmod/chown semantics, and old handle may leak on partial failure paths osquery/utils/pidfile/pidfile_windows.cpp:88
55 🟢 95 high original_mnt_fd_ closed unconditionally in destructor even when never opened (-1) osquery/worker/ipc/linux/linux_table_container_ipc.cpp:139

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.


Run: https://product-hub.flamingo.so/admin/code-review
Run id: f72ecbf3-9b66-4d6c-b85c-8e5d888e7d88

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

ClickUp task: CU-86akhf8u2 Osquery review findings sweep (1 PRs)

flamingo Bot added 30 commits September 14, 2026 05:21
@flamingo flamingo Bot changed the title fix(adhoc-sweep-fixes): 56 review findings across 40 files fix(adhoc-sweep-fixes): CU-86akhf8u2 56 review findings across 40 files Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants