Skip to content

cpp: model BDE bdlbb::Blob byte-buffer taint flow - #22455

Open
kumarak wants to merge 2 commits into
github:mainfrom
trail-of-forks:kumarak/cpp-bdlbb-blob-models
Open

cpp: model BDE bdlbb::Blob byte-buffer taint flow#22455
kumarak wants to merge 2 commits into
github:mainfrom
trail-of-forks:kumarak/cpp-bdlbb-blob-models

Conversation

@kumarak

@kumarak kumarak commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Add flow summaries for the BDE segmented byte buffer BloombergLP::bdlbb::Blob so taint reaches a blob's payload bytes:

  • Accessor chain: Blob::buffer taints the returned BlobBuffer, and BlobBuffer::data/buffer taint the bytes.
  • bdlbb::BlobUtil::copy and getContiguousRangeOrCopy propagate taint between a blob and a flat buffer in both directions.

This unblocks blob-carried sources such as bmqa::Message::getData, whose payload was previously stranded on the opaque Blob object. Not a duplicate; the bdlbb namespace had no coverage. Verified with a BloombergLP::bdlbb-shaped stub in the dataflow external-models harness.

Copilot AI balanced review requested due to automatic review settings August 27, 2026 19:52
@kumarak
kumarak requested a review from a team as a code owner August 27, 2026 19:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds BDE bdlbb::Blob taint-flow models for payload access and copying.

Changes:

  • Models Blob and BlobBuffer accessors.
  • Models BlobUtil copy operations.
  • Adds external-model tests and release notes.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
cpp/ql/lib/ext/bdlbb.model.yml Defines flow summaries.
cpp/ql/test/library-tests/dataflow/external-models/bdlbb.cpp Adds test stubs and cases.
cpp/ql/test/library-tests/dataflow/external-models/flow.expected Updates expected flow results.
cpp/ql/test/library-tests/dataflow/external-models/steps.expected Updates expected summary steps.
cpp/ql/lib/change-notes/2026-08-27-bdlbb-blob-models.md Documents the analysis improvement.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance
# Accessor chain: a tainted blob taints its buffers, and a tainted buffer taints its bytes.
- ["BloombergLP::bdlbb", "Blob", true, "buffer", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
- ["BloombergLP::bdlbb", "BlobBuffer", true, "data", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we at least want to add BlobBuffer::buffer() here. bsl::shared_ptr<char> seems out-of-scope.

@jketema jketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again only looked at this briefly.

Comment thread cpp/ql/lib/ext/bdlbb.model.yml Outdated
Add flow summaries for the BDE segmented byte buffer
BloombergLP::bdlbb::Blob so taint reaches a blob's payload bytes:

- Accessor chain: Blob::buffer taints the returned BlobBuffer, and
  BlobBuffer::data/buffer taint the bytes.
- bdlbb::BlobUtil::copy and getContiguousRangeOrCopy propagate taint
  between a blob and a flat buffer in both directions.

This unblocks blob-carried sources such as bmqa::Message::getData, whose
payload was previously stranded on the opaque Blob object. Not a
duplicate; the bdlbb namespace had no coverage. Verified with a
BloombergLP::bdlbb-shaped stub in the dataflow external-models harness.
@kumarak
kumarak force-pushed the kumarak/cpp-bdlbb-blob-models branch from 420d1c1 to 8701ce9 Compare August 30, 2026 21:04

@jketema jketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would again reduce the amount of comments. Otherwise this LGTM.

Comment thread cpp/ql/lib/ext/bdlbb.model.yml Outdated
Comment thread cpp/ql/lib/ext/bdlbb.model.yml Outdated
Comment thread cpp/ql/lib/ext/bdlbb.model.yml Outdated
Comment thread cpp/ql/lib/change-notes/2026-08-27-bdlbb-blob-models.md Outdated
@kumarak
kumarak requested a review from jketema August 31, 2026 12:41
Address review feedback on the bdlbb::Blob models:

- Add a summary for BlobBuffer::buffer(), which returns the
  bsl::shared_ptr<char> that owns the bytes. No shared_ptr rows are
  needed: SmartPointer.qll already covers bsl::shared_ptr::get().
- Add a harness case that reads through blob.buffer(0).buffer().get().
- Shorten the section comments in bdlbb.model.yml and the change note.

@jketema jketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm running some more (internal) testing before merging this.

@jketema

jketema commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

I'm seeing the following test failure:

2026-09-03T09:03:46.2238541Z --- expected
2026-09-03T09:03:46.2268066Z +++ actual
2026-09-03T09:03:46.2298528Z @@ -1962,6 +1962,15 @@
2026-09-03T09:03:46.2309675Z  | (BUF_MEM *,size_t) |  | BUF_MEM_grow | 1 | size_t |
2026-09-03T09:03:46.2370522Z Extracting test database in /home/runner/work/semmle-code/semmle-code/semmlecode-cpp-tests/extractor_tests/literal_operator.
2026-09-03T09:03:46.2372489Z  | (BUF_MEM *,size_t) |  | BUF_MEM_grow_clean | 0 | BUF_MEM * |
2026-09-03T09:03:46.2440778Z  | (BUF_MEM *,size_t) |  | BUF_MEM_grow_clean | 1 | size_t |
2026-09-03T09:03:46.2489630Z +| (Blob *,int,const Blob &,int,int) | BlobUtil | copy | 0 | Blob * |
2026-09-03T09:03:46.2502983Z +| (Blob *,int,const Blob &,int,int) | BlobUtil | copy | 1 | int |
2026-09-03T09:03:46.2529494Z +| (Blob *,int,const Blob &,int,int) | BlobUtil | copy | 2 | const Blob & |
2026-09-03T09:03:46.2589236Z +| (Blob *,int,const Blob &,int,int) | BlobUtil | copy | 3 | int |
2026-09-03T09:03:46.2591015Z +| (Blob *,int,const Blob &,int,int) | BlobUtil | copy | 4 | int |
2026-09-03T09:03:46.2592714Z +| (Blob *,int,const char *,int) | BlobUtil | copy | 0 | Blob * |
2026-09-03T09:03:46.2594387Z +| (Blob *,int,const char *,int) | BlobUtil | copy | 1 | int |
2026-09-03T09:03:46.2597149Z +| (Blob *,int,const char *,int) | BlobUtil | copy | 2 | const char * |
2026-09-03T09:03:46.2630147Z +| (Blob *,int,const char *,int) | BlobUtil | copy | 3 | int |
2026-09-03T09:03:46.2649905Z Extracting test database in /home/runner/work/semmle-code/semmle-code/semmlecode-cpp-tests/extractor_tests/include_multiline.
2026-09-03T09:03:46.2670031Z  | (BrotliBitReader *const,uint64_t,uint64_t *) |  | BrotliSafeReadBits32Slow | 0 | BrotliBitReader *const |
2026-09-03T09:03:46.2689151Z  | (BrotliBitReader *const,uint64_t,uint64_t *) |  | BrotliSafeReadBits32Slow | 1 | uint64_t |
2026-09-03T09:03:46.2730317Z  | (BrotliBitReader *const,uint64_t,uint64_t *) |  | BrotliSafeReadBits32Slow | 2 | uint64_t * |
2026-09-03T09:03:46.2758595Z @@ -13127,6 +13136,10 @@
2026-09-03T09:03:46.2778981Z  | (char *,char,char **) |  | __old_strtok_r_1c | 0 | char * |
2026-09-03T09:03:46.2829364Z  | (char *,char,char **) |  | __old_strtok_r_1c | 1 | char |
2026-09-03T09:03:46.2839624Z  | (char *,char,char **) |  | __old_strtok_r_1c | 2 | char ** |
2026-09-03T09:03:46.2900142Z +| (char *,const Blob &,int,int) | BlobUtil | copy | 0 | char * |
2026-09-03T09:03:46.2939474Z +| (char *,const Blob &,int,int) | BlobUtil | copy | 1 | const Blob & |
2026-09-03T09:03:46.2978946Z +| (char *,const Blob &,int,int) | BlobUtil | copy | 2 | int |
2026-09-03T09:03:46.2989624Z +| (char *,const Blob &,int,int) | BlobUtil | copy | 3 | int |
2026-09-03T09:03:46.2991509Z  | (char *,const char *) |  | xstrdup | 0 | char * |
2026-09-03T09:03:46.2993051Z  | (char *,const char *) |  | xstrdup | 1 | const char * |
2026-09-03T09:03:46.2995374Z  | (char *,const char **,const char **,const char **,const char **,const char **) |  | _nl_explode_name | 0 | char * |
2026-09-03T09:03:46.3003703Z ##[error][1/3] [731/2863 comp 44.2s eval 1.3s] FAILED(RESULT) /home/runner/work/semmle-code/semmle-code/ql/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.ql

@jketema

jketema commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Internal testing showed nothing out of the ordinary. So if you fix the test, then this can be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants