cpp: model BDE bdlbb::Blob byte-buffer taint flow #13
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * Added flow summaries for the BDE `BloombergLP::bdlbb::Blob` segmented byte buffer. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Model of the BDE bdlbb::Blob segmented byte buffer (BloombergLP::bdlbb). | ||
| # Lets taint reach a blob's payload bytes, e.g. a message body filled by bmqa::Message::getData. | ||
| extensions: | ||
| - addsTo: | ||
| pack: codeql/cpp-all | ||
| extensible: summaryModel | ||
| data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance | ||
| # Accessor chain | ||
| - ["BloombergLP::bdlbb", "Blob", true, "buffer", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"] | ||
| - ["BloombergLP::bdlbb", "BlobBuffer", true, "data", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"] | ||
| - ["BloombergLP::bdlbb", "BlobBuffer", true, "buffer", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"] | ||
| # BlobUtil read-out | ||
|
There was a problem hiding this comment. Shipwright · HIGH The BlobUtil::copy overload taking (char *,const Blob &,int,int) is modeled as Argument[*1] -> Argument[*0] with no length or position constraints. Impact: The BlobUtil::copy overload taking (char *,const Blob &,int,int) is modeled as Argument[*1] -> Argument[*0] with no length or position constraints. A caller that copies only a zero-length or out-of-range slice will still have the entire destination buffer marked tainted, which can create misleading flows in code that partially initializes a buffer. Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| - ["BloombergLP::bdlbb", "BlobUtil", true, "copy", "(char *,const Blob &,int,int)", "", "Argument[*1]", "Argument[*0]", "taint", "manual"] | ||
|
There was a problem hiding this comment. Shipwright · HIGH The model for BlobUtil::getContiguousRangeOrCopy has two entries with the same empty signature, one mapping Argument[*1] to Argument[*0] and one mapping Argument[*1] to ReturnValue Impact: The model for BlobUtil::getContiguousRangeOrCopy has two entries with the same empty signature, one mapping Argument[*1] to Argument[*0] and one mapping Argument[1] to ReturnValue[]. If the real function returns a pointer into the blob when the buffer is contiguous and copies into dstBuffer otherwise, the unconditional Argument[*1] -> Argument[*0] summary will propagate taint to dstBuffer even when the function… Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| - ["BloombergLP::bdlbb", "BlobUtil", true, "getContiguousRangeOrCopy", "", "", "Argument[*1]", "Argument[*0]", "taint", "manual"] | ||
| - ["BloombergLP::bdlbb", "BlobUtil", true, "getContiguousRangeOrCopy", "", "", "Argument[*1]", "ReturnValue[*]", "taint", "manual"] | ||
| # BlobUtil write-in | ||
|
There was a problem hiding this comment. Shipwright · HIGH The model marks BlobUtil::copy(Blob *,int,const char *,int) as Argument[*2] -> Argument[*0] with no sanitization or length validation. Impact: The model marks BlobUtil::copy(Blob *,int,const char *,int) as Argument[*2] -> Argument[*0] with no sanitization or length validation. This treats any caller-supplied source buffer as fully tainted into the destination blob, which is appropriate for taint tracking but will flag every write path, including internal buffer initialization, as attacker-controlled unless downstream models add sanitizers. Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| - ["BloombergLP::bdlbb", "BlobUtil", true, "copy", "(Blob *,int,const char *,int)", "", "Argument[*2]", "Argument[*0]", "taint", "manual"] | ||
| - ["BloombergLP::bdlbb", "BlobUtil", true, "copy", "(Blob *,int,const Blob &,int,int)", "", "Argument[*2]", "Argument[*0]", "taint", "manual"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
|
|
||
| // --- stub library headers --- | ||
|
|
||
| namespace bsl { | ||
| typedef unsigned long size_t; | ||
| template <class T> class allocator {}; | ||
| template<class charT> struct char_traits {}; | ||
| template<class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> > | ||
| class basic_string { | ||
| public: | ||
| basic_string(const charT* s, const Allocator& a = Allocator()); | ||
| const charT* data() const; | ||
| size_t size() const; | ||
| }; | ||
| typedef basic_string<char> string; | ||
| template <class T> class shared_ptr { | ||
| public: | ||
| T *get() const; | ||
| }; | ||
| } | ||
|
|
||
| namespace BloombergLP { | ||
| namespace bdlbb { | ||
| class BlobBuffer { | ||
| public: | ||
| char *data() const; | ||
| bsl::shared_ptr<char> &buffer(); | ||
| const bsl::shared_ptr<char> &buffer() const; | ||
| }; | ||
|
|
||
| class Blob { | ||
| public: | ||
| const BlobBuffer &buffer(int index) const; | ||
| }; | ||
|
|
||
| struct BlobUtil { | ||
| static void copy(char *dstBuffer, const Blob &srcBlob, int position, int length); | ||
| static void copy(Blob *dstBlob, int dstOffset, const char *srcBuffer, int length); | ||
| static void copy(Blob *dstBlob, int dstOffset, const Blob &srcBlob, int srcOffset, | ||
| int length); | ||
| static char *getContiguousRangeOrCopy(char *dstBuffer, const Blob &srcBlob, int position, | ||
| int length, int alignment); | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| // --- test code --- | ||
|
|
||
| char *source(); | ||
| void sink(char); | ||
|
|
||
| // A blob populated from a tainted buffer taints the bytes read back out of it. | ||
| void test_BlobUtil_copy() { | ||
| bsl::string s(source()); | ||
| BloombergLP::bdlbb::Blob blob; | ||
| BloombergLP::bdlbb::BlobUtil::copy(&blob, 0, s.data(), s.size()); | ||
| char dst[16]; | ||
| BloombergLP::bdlbb::BlobUtil::copy(dst, blob, 0, 16); | ||
| sink(*dst); // $ ir | ||
| } | ||
|
|
||
| void test_accessor_chain() { | ||
| bsl::string s(source()); | ||
| BloombergLP::bdlbb::Blob blob; | ||
| BloombergLP::bdlbb::BlobUtil::copy(&blob, 0, s.data(), s.size()); | ||
| const char *p = blob.buffer(0).data(); | ||
| sink(*p); // $ ir | ||
| } | ||
|
|
||
| // The get() step comes from the built-in smart pointer model, not from bdlbb.model.yml. | ||
|
There was a problem hiding this comment. Shipwright · LOW The test stubs declare BlobBuffer::buffer() returning bsl::shared_ptr<char>&, but the model comment says the get() step comes from the built-in smart pointer model. Impact: The test stubs declare BlobBuffer::buffer() returning bsl::shared_ptr<char>&, but the model comment says the get() step comes from the built-in smart pointer model. A new contributor cannot tell from the test file alone why the shared_ptr path is expected to work or whether the non-const overload is intentionally covered. Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| void test_accessor_chain_shared_ptr() { | ||
| bsl::string s(source()); | ||
| BloombergLP::bdlbb::Blob blob; | ||
| BloombergLP::bdlbb::BlobUtil::copy(&blob, 0, s.data(), s.size()); | ||
| const char *p = blob.buffer(0).buffer().get(); | ||
| sink(*p); // $ ir | ||
| } | ||
|
|
||
| void test_getContiguousRangeOrCopy() { | ||
| bsl::string s(source()); | ||
| BloombergLP::bdlbb::Blob blob; | ||
| BloombergLP::bdlbb::BlobUtil::copy(&blob, 0, s.data(), s.size()); | ||
| char dst[16]; | ||
| char *r = BloombergLP::bdlbb::BlobUtil::getContiguousRangeOrCopy(dst, blob, 0, 16, 1); | ||
| sink(*r); // $ ir | ||
| } | ||
|
|
||
| // A blob copied into another blob carries the taint across. | ||
| void test_BlobUtil_copy_blob_to_blob() { | ||
| bsl::string s(source()); | ||
| BloombergLP::bdlbb::Blob src; | ||
| BloombergLP::bdlbb::BlobUtil::copy(&src, 0, s.data(), s.size()); | ||
| BloombergLP::bdlbb::Blob dst; | ||
| BloombergLP::bdlbb::BlobUtil::copy(&dst, 0, src, 0, 16); | ||
| char out[16]; | ||
| BloombergLP::bdlbb::BlobUtil::copy(out, dst, 0, 16); | ||
| sink(*out); // $ ir | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shipwright · HIGH
The model file uses empty signatures for Blob::buffer, BlobBuffer::data, BlobBuffer::buffer, and getContiguousRangeOrCopy.
Impact: The model file uses empty signatures for Blob::buffer, BlobBuffer::data, BlobBuffer::buffer, and getContiguousRangeOrCopy. This makes the model match any overload or future overload with the same name, and gives a reader no way to tell which API surface is actually covered without cross-referencing the test stubs.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.