From 14439fb4387a0e9143c93bb954c2a164d269931f Mon Sep 17 00:00:00 2001 From: audristroyer Date: Fri, 12 Jun 2026 15:53:22 -0400 Subject: [PATCH 1/4] security(sqlitedb): escape interpolated ids in double-quoted SQL literals (audit 6.1-3) Several sqlitedb queries interpolate branch_id / doc_id / document_id / filename directly into SQL string literals delimited by double quotes, e.g. ['... WHERE doc_id="' document_id '"']. run_sql_query() does not forward bind parameters to mksqlite (it calls do_run_sql_query without varargin), so these values cannot be passed as '?' placeholders; a value containing a double quote could break out of the literal and inject SQL (e.g. a crafted branch name or document id). Add a private escapeSqlLiteral helper that doubles embedded double quotes (inside a double-quoted token SQLite reads "" as an escaped "), mirroring the DID-python _sql_escape (which doubles single quotes for its single-quoted literals), and apply it at all 15 value-interpolation sites (do_add_branch / do_delete_branch / do_get_branch_parent / do_get_sub_branches / do_get_doc_ids / read / remove_doc / open_doc / file path lookups). The existing parameterized run_sql_noOpen('... =?', val) call sites are unchanged. Lockstep with the DID-python field-name validation (same audit item 6.1-3). Authored without a local MATLAB runtime; needs MATLAB to validate/run. --- src/did/+did/+implementations/sqlitedb.m | 47 +++++++++++++++++------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/src/did/+did/+implementations/sqlitedb.m b/src/did/+did/+implementations/sqlitedb.m index 01f0e5b..cbd0929 100644 --- a/src/did/+did/+implementations/sqlitedb.m +++ b/src/did/+did/+implementations/sqlitedb.m @@ -113,6 +113,25 @@ function delete(this_obj) % disposed when this method returns, using the onCleanup mechanism end % do_run_sql_query() + function s = escapeSqlLiteral(~, s) + % escapeSqlLiteral - escape a value for a double-quoted SQL literal + % + % S = escapeSqlLiteral(THIS_OBJ, S) + % + % Several queries below interpolate identifiers (branch_id, doc_id, + % filename) into SQL string literals delimited by double quotes, + % e.g. ['... WHERE doc_id="' document_id '"']. run_sql_query() does + % not forward bind parameters to mksqlite (it calls + % do_run_sql_query without varargin), so those values cannot be + % passed as '?' placeholders and must be escaped to prevent SQL + % injection from a crafted id/branch name. Inside a double-quoted + % token SQLite treats "" as an escaped ", so doubling the double + % quotes neutralizes any attempt to break out of the literal. This + % mirrors the DID-python _sql_escape (which doubles single quotes + % for its single-quoted literals). + s = strrep(char(s), '"', '""'); + end % escapeSqlLiteral() + function branch_ids = do_get_branch_ids(this_obj) % do_get_branch_ids - return all unique branch ids in the database % @@ -153,7 +172,7 @@ function do_add_branch(this_obj, branch_id, parent_branch_id, varargin) this_obj.insert_into_table('branches', 'branch_id,parent_id,timestamp', branch_id, parent_branch_id, tnow); % Duplicate the docs from parent branch to the newly-created branch - sqlStr = ['SELECT doc_idx FROM branch_docs WHERE branch_id="' parent_branch_id '"']; + sqlStr = ['SELECT doc_idx FROM branch_docs WHERE branch_id="' this_obj.escapeSqlLiteral(parent_branch_id) '"']; data = this_obj.run_sql_noOpen(sqlStr); if ~isempty(data) doc_idx = [data.doc_idx]; @@ -176,11 +195,11 @@ function do_delete_branch(this_obj, branch_id, varargin) if ~isempty(doc_ids) % Remove all documents from the branch_docs table % TODO: also delete records of unreferenced docs ??? - this_obj.run_sql_query(['DELETE FROM branch_docs WHERE branch_id="' branch_id '"']); + this_obj.run_sql_query(['DELETE FROM branch_docs WHERE branch_id="' this_obj.escapeSqlLiteral(branch_id) '"']); end % Now delete the branch record - this_obj.run_sql_query(['DELETE FROM branches WHERE branch_id="' branch_id '"']); + this_obj.run_sql_query(['DELETE FROM branches WHERE branch_id="' this_obj.escapeSqlLiteral(branch_id) '"']); end % do_delete_branch() function parent_branch_id = do_get_branch_parent(this_obj, branch_id, varargin) @@ -190,7 +209,7 @@ function do_delete_branch(this_obj, branch_id, varargin) % % Returns the ID of the parent branch for the specified BRANCH_ID. - sqlStr = ['SELECT parent_id FROM branches WHERE branch_id="' branch_id '"']; + sqlStr = ['SELECT parent_id FROM branches WHERE branch_id="' this_obj.escapeSqlLiteral(branch_id) '"']; data = this_obj.run_sql_query(sqlStr); if isempty(data) parent_branch_id = ''; @@ -219,7 +238,7 @@ function do_delete_branch(this_obj, branch_id, varargin) % Returns a cell array of IDs of sub-branches of the specified BRANCH_ID. % If BRANCH_ID has no sub-branches, an empty cell array is returned. - sqlStr = ['SELECT branch_id FROM branches WHERE parent_id="' branch_id '"']; + sqlStr = ['SELECT branch_id FROM branches WHERE parent_id="' this_obj.escapeSqlLiteral(branch_id) '"']; data = this_obj.run_sql_query(sqlStr); if isempty(data) branch_ids = {}; @@ -241,7 +260,7 @@ function do_delete_branch(this_obj, branch_id, varargin) if nargin > 1 && ~isempty(branch_id) sqlStr = ['SELECT docs.doc_id FROM docs,branch_docs' ... ' WHERE docs.doc_idx = branch_docs.doc_idx' ... - ' AND branch_id="' branch_id '"']; + ' AND branch_id="' this_obj.escapeSqlLiteral(branch_id) '"']; else sqlStr = 'SELECT docs.doc_id FROM docs'; end @@ -432,7 +451,7 @@ function do_add_doc(this_obj, document_obj, branch_id, options) %document_obj = did.document(doc); % Run the SQL query in the database - query_str = ['SELECT json_code FROM docs WHERE doc_id="' document_id '"']; + query_str = ['SELECT json_code FROM docs WHERE doc_id="' this_obj.escapeSqlLiteral(document_id) '"']; data = this_obj.run_sql_query(query_str); % Process missing document results @@ -500,8 +519,8 @@ function do_remove_doc(this_obj, document_id, branch_id, options) % Handle case of missing document sqlStr = ['SELECT docs.doc_idx FROM docs,branch_docs ' ... ' WHERE docs.doc_idx = branch_docs.doc_idx ' ... - ' AND branch_id="' branch_id '"' ... - ' AND doc_id="' doc_id '"']; + ' AND branch_id="' this_obj.escapeSqlLiteral(branch_id) '"' ... + ' AND doc_id="' this_obj.escapeSqlLiteral(doc_id) '"']; %doc_id = [doc_id '/' branch_id]; data = this_obj.run_sql_noOpen(sqlStr); if isempty(data) @@ -521,7 +540,7 @@ function do_remove_doc(this_obj, document_id, branch_id, options) doc_idx = data(1).doc_idx; % Remove the document from the branch_docs table - this_obj.run_sql_noOpen(['DELETE FROM branch_docs WHERE branch_id="' branch_id '" AND doc_idx=?'], doc_idx); + this_obj.run_sql_noOpen(['DELETE FROM branch_docs WHERE branch_id="' this_obj.escapeSqlLiteral(branch_id) '" AND doc_idx=?'], doc_idx); % TODO - remove all document records if no branch references remain? %{ @@ -579,10 +598,10 @@ function do_remove_doc(this_obj, document_id, branch_id, options) % Get the cached filepath to the specified document query_str = ['SELECT cached_location,orig_location,uid,type ' ... ' FROM docs,files ' ... - ' WHERE docs.doc_id="' document_id '" ' ... + ' WHERE docs.doc_id="' this_obj.escapeSqlLiteral(document_id) '" ' ... ' AND files.doc_idx=docs.doc_idx']; if nargin > 2 && ~isempty(filename) - query_str = [query_str ' AND files.filename="' filename '"']; + query_str = [query_str ' AND files.filename="' this_obj.escapeSqlLiteral(filename) '"']; else error('DID:SQLITEDB:open','The requested filename must be specified in open_doc()'); %filename = ''; % used in catch block below @@ -713,10 +732,10 @@ function tryCustomFileHandler(customFileHandler, destPath, sourcePath, file_type % Get the cached filepath to the specified document query_str = ['SELECT cached_location,orig_location,uid,type ' ... ' FROM docs,files ' ... - ' WHERE docs.doc_id="' document_id '" ' ... + ' WHERE docs.doc_id="' this_obj.escapeSqlLiteral(document_id) '" ' ... ' AND files.doc_idx=docs.doc_idx']; if nargin > 2 && ~isempty(filename) - query_str = [query_str ' AND files.filename="' filename '"']; + query_str = [query_str ' AND files.filename="' this_obj.escapeSqlLiteral(filename) '"']; else error('DID:SQLITEDB:open','The requested filename must be specified in check_exist_doc()'); end From be7462c51e065dd000f66832be9937204cbd087d Mon Sep 17 00:00:00 2001 From: audristroyer Date: Fri, 12 Jun 2026 15:55:30 -0400 Subject: [PATCH 2/4] docs: add audit remediation results for the DID-matlab branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maps the §6.1 finding addressed here (SQL-literal escaping, 6.1-3) to its commit, notes that isa (6.1-1) and the sqlite indexes (6.1-4) needed no MATLAB change (MATLAB is the correct reference / already has them; Python caught up), and records the timestamp-format DECISION (§6.1-2/§7.3-13: datenum vs epoch seconds in the same column) as a cross-client choice deliberately NOT changed here, pending joint sign-off. Reiterates author-not-run + the lockstep merge with audit/did-python-2026-06. --- docs/Audit_Remediation_Results_2026-06-12.md | 46 ++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 docs/Audit_Remediation_Results_2026-06-12.md diff --git a/docs/Audit_Remediation_Results_2026-06-12.md b/docs/Audit_Remediation_Results_2026-06-12.md new file mode 100644 index 0000000..2441c24 --- /dev/null +++ b/docs/Audit_Remediation_Results_2026-06-12.md @@ -0,0 +1,46 @@ +# DID-matlab Audit Remediation — Results (2026-06-12) + +Branch `audit/did-matlab-2026-06`, off `origin/main` (`03b0f7f`). This is the +MATLAB half of the **DID lockstep**; the Python half is +`audit/did-python-2026-06`. + +> **⚠️ Author-not-run.** Authored without a local MATLAB runtime. Please run the +> DID-matlab + symmetry test suites before merging. + +## Findings addressed (audit §6.1) + +| # | Severity | Commit | Summary | +|---|----------|--------|---------| +| 6.1-3 | Medium | `14439fb` | **SQL injection via interpolated ids.** Several `sqlitedb` queries interpolate `branch_id` / `doc_id` / `document_id` / `filename` directly into double-quoted SQL string literals, e.g. `['... WHERE doc_id="' document_id '"']`. `run_sql_query()` does not forward bind parameters to mksqlite (it calls `do_run_sql_query` without `varargin`), so those values cannot be passed as `?` placeholders; a value containing a double quote could break out of the literal and inject SQL. Added a private `escapeSqlLiteral` helper that doubles embedded double quotes (inside a double-quoted token SQLite reads `""` as an escaped `"`), mirroring DID-python's `_sql_escape`, and applied it at all 15 value-interpolation sites. The already-parameterized `run_sql_noOpen('... =?', val)` calls are unchanged. | + +### isa (6.1-1) — no MATLAB change needed + +MATLAB's `isa` (via `doc2sql` `meta.class`/`meta.superclass` + the SQL translation) is +the **correct reference**; the divergence was on the Python side, which has been brought +to parity (`audit/did-python-2026-06`). No DID-matlab isa change is required; the two are +merged together so a symmetry run stays consistent. + +### sqlite indexes (6.1-4) — already present in MATLAB + +The audit item is "Python omits MATLAB's sqlite indexes." MATLAB already creates them; +the Python side added the equivalents. No MATLAB change. + +## DECISION required — do not merge a serialization change without sign-off + +**§6.1-2 / §7.3-13: the `timestamp` column meaning diverges across languages and is NOT +changed here.** DID-matlab writes MATLAB `now` (datenum — days since year 0) into the +`docs`/`branches`/`branch_docs` `timestamp REAL` column (`sqlitedb.m` `do_add_branch`, +add-doc, add-to-branch); DID-python writes `time.time()` (Unix epoch seconds) into the +same column. Cross-client `lessthan`/`greaterthan` comparisons on `timestamp` silently +break, and the cloud backend stores the value verbatim. + +This is a **cross-client format decision**, not a one-sided patch — changing the +serialization on either side alone would break the other and any already-stored data. It +needs an explicit decision (proposed: **ISO-8601 TEXT**, or Unix epoch seconds, with a +documented one-time conversion for existing rows) made jointly for DID-python, +DID-matlab, and the cloud backend. Left untouched pending that decision. + +## Lockstep / merge + +Merge with `audit/did-python-2026-06` (the Python field-name validation is the same audit +item 6.1-3). VH-Lab fork-and-PR: a DID-matlab fork is created at review time. From 02c54b5464220995a61ed8698126bfd73baaa0a9 Mon Sep 17 00:00:00 2001 From: audristroyer Date: Fri, 12 Jun 2026 18:43:21 -0400 Subject: [PATCH 3/4] docs: correct escapeSqlLiteral site count (14, not 15) Adversarial review counted 14 escapeSqlLiteral call sites in sqlitedb.m, not 15 (the earlier count of 15 included a docstring-example occurrence that was later reverted). All vulnerable double-quoted interpolations are still covered. --- docs/Audit_Remediation_Results_2026-06-12.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Audit_Remediation_Results_2026-06-12.md b/docs/Audit_Remediation_Results_2026-06-12.md index 2441c24..035b6fd 100644 --- a/docs/Audit_Remediation_Results_2026-06-12.md +++ b/docs/Audit_Remediation_Results_2026-06-12.md @@ -11,7 +11,7 @@ MATLAB half of the **DID lockstep**; the Python half is | # | Severity | Commit | Summary | |---|----------|--------|---------| -| 6.1-3 | Medium | `14439fb` | **SQL injection via interpolated ids.** Several `sqlitedb` queries interpolate `branch_id` / `doc_id` / `document_id` / `filename` directly into double-quoted SQL string literals, e.g. `['... WHERE doc_id="' document_id '"']`. `run_sql_query()` does not forward bind parameters to mksqlite (it calls `do_run_sql_query` without `varargin`), so those values cannot be passed as `?` placeholders; a value containing a double quote could break out of the literal and inject SQL. Added a private `escapeSqlLiteral` helper that doubles embedded double quotes (inside a double-quoted token SQLite reads `""` as an escaped `"`), mirroring DID-python's `_sql_escape`, and applied it at all 15 value-interpolation sites. The already-parameterized `run_sql_noOpen('... =?', val)` calls are unchanged. | +| 6.1-3 | Medium | `14439fb` | **SQL injection via interpolated ids.** Several `sqlitedb` queries interpolate `branch_id` / `doc_id` / `document_id` / `filename` directly into double-quoted SQL string literals, e.g. `['... WHERE doc_id="' document_id '"']`. `run_sql_query()` does not forward bind parameters to mksqlite (it calls `do_run_sql_query` without `varargin`), so those values cannot be passed as `?` placeholders; a value containing a double quote could break out of the literal and inject SQL. Added a private `escapeSqlLiteral` helper that doubles embedded double quotes (inside a double-quoted token SQLite reads `""` as an escaped `"`), mirroring DID-python's `_sql_escape`, and applied it at all 14 value-interpolation sites. The already-parameterized `run_sql_noOpen('... =?', val)` calls are unchanged. | ### isa (6.1-1) — no MATLAB change needed From 80c2a81e2586d95fb315b00f6d3aa8f83eededfc Mon Sep 17 00:00:00 2001 From: audristroyer Date: Fri, 12 Jun 2026 19:49:56 -0400 Subject: [PATCH 4/4] docs: add reviewer/next-agent context banner (part of the 9-PR 2026-06 ecosystem audit; lockstep + deferrals) --- docs/Audit_Remediation_Results_2026-06-12.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/Audit_Remediation_Results_2026-06-12.md b/docs/Audit_Remediation_Results_2026-06-12.md index 035b6fd..d84642a 100644 --- a/docs/Audit_Remediation_Results_2026-06-12.md +++ b/docs/Audit_Remediation_Results_2026-06-12.md @@ -1,5 +1,11 @@ # DID-matlab Audit Remediation — Results (2026-06-12) +> **Context for a reviewer / next agent.** One of **9 coordinated PRs** in the 2026-06 NDI +> ecosystem audit; **none are merged.** This repo's PR: **VH-Lab/DID-matlab#146** — **merge +> together with the DID-python PR VH-Lab/DID-python#23** (same audit item §6.1). +> **Author-not-run** — run the DID-matlab + symmetry suites before merge. The one deferral +> is the **`timestamp` format DECISION** (see "DECISION required" below). + Branch `audit/did-matlab-2026-06`, off `origin/main` (`03b0f7f`). This is the MATLAB half of the **DID lockstep**; the Python half is `audit/did-python-2026-06`.