SCJ in F Prime.
From analysis to accepted changes.

Merged 2026-06-09 · 96187b7

Svc/FileUplink/File.cpp

@@ -20,6 +20,10 @@ namespace Svc {
 Os::File::Status FileUplink::File::open(const Fw::FilePacket::StartPacket& startPacket) {
     const U32 length = startPacket.getDestinationPath().getLength();
     char path[Fw::FilePacket::PathName::MAX_LENGTH + 1];
+    // Ensure that length is not greater that the size of the variable we will copy into
+    if (length >= sizeof(path)) {
+        return Os::File::Status::BAD_SIZE;
+    }
     memcpy(path, startPacket.getDestinationPath().getValue(), length);
     path[length] = 0;
     Fw::LogStringArg logStringArg(path);

Merged 2026-08-04 · 1e91573

Svc/DpCompressProc/DpCompressProc.cpp

@@ -53,6 +53,12 @@ void DpCompressProc ::procRequest_handler(FwIndexType portNum, Fw::Buffer& fwBuf
         return;
     }

+    // Check that the buffer is large enough to hold a data product packet
+    if (fwBuffer.getSize() < Fw::DpContainer::MIN_PACKET_SIZE) {
+        this->log_WARNING_HI_BufferTooSmallForPacket(fwBuffer.getSize(), Fw::DpContainer::MIN_PACKET_SIZE);
+        return;
+    }
+
     Fw::DpContainer container(0, fwBuffer);
     const Fw::SerializeStatus desStat = container.deserializeHeader();
     if (desStat != Fw::FW_SERIALIZE_OK) {

Svc/DpCompressProc/DpCompressProc.fpp

@@ -32,6 +32,14 @@ module Svc {
             format "Received buffer of size {}; deserialization of container header failed with error code {}" \
             throttle 10

+        @ Received buffer is too small to hold a data product packet
+        event BufferTooSmallForPacket(buffer_size: FwSizeType @< The incoming buffer size
+                                      min_size: FwSizeType @< The minimum required size
+                                      ) \
+            severity warning high \
+            format "Received buffer has size {}; minimum required size is {}" \
+            throttle 10
+
         @ Record ID used to mark compressed records in a data product
         product record CompressionRecord: U8 array
 

Svc/DpCompressProc/docs/sdd.md

@@ -71,6 +71,8 @@ Add requirements in the chart below
 |-----------------------|--------------------------------------------------------------------|
 | CompressionComplete   | DIAGNOSTIC event when DpCompressProc compresses a buffer           |
 | DidNotCompress        | ACTIVITY\_LO event when DpCompressProc can not compress any chunks |
+| InvalidHeader         | WARNING\_HI event when the container header cannot be deserialized |
+| BufferTooSmallForPacket | WARNING\_HI event when the incoming buffer is smaller than `Fw::DpContainer::MIN_PACKET_SIZE` |


 ## Detailed Documentation

Svc/DpCompressProc/test/ut/DpCompressProcTestMain.cpp

@@ -9,6 +9,10 @@
 #include "STest/STest/Random/Random.hpp"
 #include "Svc/DpCompressProc/test/ut/Rules/Testers.hpp"

+TEST(OffNominal, BufferSmallerThanMinPacketSize) {
+    Svc::Testers::procRequest.testState.test_undersized_buffer();
+}
+
 TEST(Nominal, Compressible) {
     const FwSizeType chunk_size = 4096;
     std::vector<Svc::AbstractState::Chunk> chunks = {Svc::AbstractState::Chunk(Svc::AbstractState::COMPRESSED, 0xA5),

Svc/DpCompressProc/test/ut/DpCompressProcTester.cpp

@@ -222,6 +222,45 @@ void DpCompressProcTester::test_chunks_helper(const FwSizeStoreType chunk_size,
     abstractState.success_ = true;
 }

+void DpCompressProcTester::test_undersized_buffer() {
+    // Hand procRequest a view one byte below MIN_PACKET_SIZE into a larger,
+    // well-formed allocation: every header byte is present, only the length is wrong.
+    this->clearHistory();
+    this->component.log_WARNING_HI_BufferTooSmallForPacket_ThrottleClear();
+
+    paramSet_CHUNK_SIZE(4096, Fw::ParamValid::VALID);
+    paramSet_ENABLE(Fw::Enabled::ENABLED, Fw::ParamValid::VALID);
+    this->component.loadParameters();
+
+    const FwSizeType backing_size = Fw::DpContainer::MIN_PACKET_SIZE + 1;
+    U8* mem = new U8[backing_size]();
+
+    Fw::Buffer backing_buf(mem, backing_size);
+    Fw::DpContainer container(0, backing_buf);
+    container.setDataSize(1);
+    container.serializeHeader();
+
+    // One byte short of the minimum
+    const FwSizeType short_size = Fw::DpContainer::MIN_PACKET_SIZE - 1;
+    Fw::Buffer short_buf(mem, short_size);
+
+    this->invoke_to_procRequest(0, short_buf);
+
+    // Rejected on size, not mistaken for a header deserialization failure
+    ASSERT_EVENTS_BufferTooSmallForPacket_SIZE(1);
+    ASSERT_EVENTS_BufferTooSmallForPacket(0, short_size, Fw::DpContainer::MIN_PACKET_SIZE);
+    ASSERT_EVENTS_InvalidHeader_SIZE(0);
+
+    // Nothing forwarded downstream, no completion claimed.
+    ASSERT_from_compressChunk_SIZE(0);
+    ASSERT_EVENTS_CompressionComplete_SIZE(0);
+
+    // Buffer left untouched.
+    ASSERT_EQ(short_buf.getSize(), short_size);
+
+    delete[] mem;
+}
+
 void DpCompressProcTester::test_chunks_disabled(const FwSizeStoreType chunk_size,
                                                 std::vector<AbstractState::Chunk> chunks) {
     Fw::Buffer container_buf = this->abstractState.build_compress_buffer(chunk_size, chunks);

Svc/DpCompressProc/test/ut/DpCompressProcTester.hpp

@@ -42,6 +42,9 @@ class DpCompressProcTester : public DpCompressProcGTestBase {

     void test_chunks_disabled(const FwSizeStoreType chunk_size, std::vector<AbstractState::Chunk> chunks);

+    //! Drive procRequest with a buffer shorter than MIN_PACKET_SIZE
+    void test_undersized_buffer();
+
   public:
     // ----------------------------------------------------------------------
     // Tests

Merged 2026-08-05 · 8818e15

.github/codeql/fprime-queries-tests/FprimeUnguardedUnsignedSubtraction/FprimeUnguardedUnsignedSubtraction.expected

@@ -0,0 +1,7 @@
+| test.cpp:30:28:30:69 | ... - ... | Unsigned subtraction from getSize() with no check that it is at least the subtracted constant; this underflows instead of going negative when the size is too small. |
+| test.cpp:78:34:78:75 | ... - ... | Unsigned subtraction from getSize() with no check that it is at least the subtracted constant; this underflows instead of going negative when the size is too small. |
+| test.cpp:106:32:106:73 | ... - ... | Unsigned subtraction from getSize() with no check that it is at least the subtracted constant; this underflows instead of going negative when the size is too small. |
+| test.cpp:124:22:124:61 | ... - ... | Unsigned subtraction from getDataSize() with no check that it is at least the subtracted constant; this underflows instead of going negative when the size is too small. |
+| test.cpp:136:28:136:69 | ... - ... | Unsigned subtraction from getSize() with no check that it is at least the subtracted constant; this underflows instead of going negative when the size is too small. |
+| test.cpp:146:28:146:69 | ... - ... | Unsigned subtraction from getSize() with no check that it is at least the subtracted constant; this underflows instead of going negative when the size is too small. |
+| test.cpp:155:28:155:69 | ... - ... | Unsigned subtraction from getSize() with no check that it is at least the subtracted constant; this underflows instead of going negative when the size is too small. |

.github/codeql/fprime-queries-tests/FprimeUnguardedUnsignedSubtraction/FprimeUnguardedUnsignedSubtraction.qlref

@@ -0,0 +1 @@
+FprimeUnguardedUnsignedSubtraction.ql

.github/codeql/fprime-queries-tests/FprimeUnguardedUnsignedSubtraction/test.cpp

@@ -0,0 +1,167 @@
+namespace Fw {
+void SwAssert(int line);
+}
+#define FW_ASSERT(cond) ((cond) ? static_cast<void>(0) : Fw::SwAssert(__LINE__))
+
+typedef unsigned long FwSizeType;
+typedef unsigned char U8;
+
+// Minimal stand-ins for the F Prime buffer/container shapes.
+class Buffer {
+  public:
+    FwSizeType getSize() const;
+    U8* getData() const;
+};
+
+class Container {
+  public:
+    static const FwSizeType MIN_PACKET_SIZE = 40;
+    static const FwSizeType DATA_OFFSET = 24;
+    FwSizeType getDataSize() const;
+};
+
+void consume(U8* data, FwSizeType size);
+
+// Violation of cpp/fprime/unguarded-unsigned-subtraction (CWE-191):
+// the buffer size comes from the caller and is never checked against
+// MIN_PACKET_SIZE, so a short buffer underflows to near SIZE_MAX.
+// This is the shape fixed in nasa/fprime#5518.
+void unguarded(const Buffer& buf) {
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+}
+
+// Compliant: FW_ASSERT is a valid check in F Prime. Projects choose their
+// own assert level; a site that asserts the invariant is not unchecked.
+void checkedByAssert(const Buffer& buf) {
+    FW_ASSERT(buf.getSize() >= Container::MIN_PACKET_SIZE);
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+}
+
+// Compliant: an explicit run-time check that survives with assertions
+// disabled.
+void guardedByEarlyReturn(const Buffer& buf) {
+    if (buf.getSize() < Container::MIN_PACKET_SIZE) {
+        return;
+    }
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+}
+
+// Compliant: the same check written the other way round.
+void guardedByIf(const Buffer& buf) {
+    if (buf.getSize() >= Container::MIN_PACKET_SIZE) {
+        consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+    }
+}
+
+// Compliant: the check is written on a local copy of the size. This is the
+// Fw::DpContainer::setBuffer shape.
+void checkedOnLocalCopy(const Buffer& buf) {
+    const FwSizeType size = buf.getSize();
+    FW_ASSERT(size >= Container::MIN_PACKET_SIZE);
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+}
+
+FwSizeType requiredSize();
+
+// Compliant: the bound is computed at run time, so the query cannot show it
+// is too small. This is the Svc::FileDownlink::sendFilePacket shape.
+void checkedAgainstComputedBound(const Buffer& buf) {
+    const FwSizeType needed = requiredSize() + Container::MIN_PACKET_SIZE;
+    FW_ASSERT(buf.getSize() >= needed);
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+}
+
+// Violation: the only comparison of the size is unrelated to the subtraction
+// and far too weak to cover it. This is the shape of the FW_ASSERT that
+// follows the subtraction in DpCompressProc; it must keep being reported.
+void unrelatedCheckAfterSubtraction(const Buffer& buf) {
+    const FwSizeType remaining = buf.getSize() - Container::MIN_PACKET_SIZE;
+    FW_ASSERT(buf.getSize() > 0);
+    consume(buf.getData(), remaining);
+}
+
+// Compliant: `> 0` is the idiomatic guard for a `- 1` subtraction. The
+// guard constant (0) is one less than the subtrahend (1), which is
+// exactly strong enough. An earlier version of this query compared the
+// two constants for equality and reported this, which was a false
+// positive -- this case pins the fix.
+void guardedByGreaterThanZero(const Buffer& buf) {
+    if (buf.getSize() > 0) {
+        consume(buf.getData(), buf.getSize() - 1);
+    }
+}
+
+// Compliant: a guard stronger than required is still a guard.
+void guardedByStrongerBound(const Buffer& buf) {
+    if (buf.getSize() < Container::MIN_PACKET_SIZE + 8) {
+        return;
+    }
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+}
+
+// Violation: a guard that is too weak to cover the subtraction. Checking
+// against 2 says nothing about whether the size reaches MIN_PACKET_SIZE.
+void guardedTooWeakly(const Buffer& buf) {
+    if (buf.getSize() > 2) {
+        consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+    }
+}
+
+// Compliant: subtracting a variable rather than a compile-time constant
+// is out of scope -- too common and too often intentional.
+void variableSubtrahend(const Buffer& buf, FwSizeType n) {
+    consume(buf.getData(), buf.getSize() - n);
+}
+
+// Compliant: not a size accessor, so the query says nothing about intent.
+FwSizeType plainArithmetic(FwSizeType value) {
+    return value - Container::MIN_PACKET_SIZE;
+}
+
+// Violation: the accessor is a different one from the guarded family,
+// and it is unchecked here.
+void unguardedDataSize(const Container& c) {
+    consume(nullptr, c.getDataSize() - Container::DATA_OFFSET);
+}
+
+// Violation: the only size comparison in the function is on a different
+// object, so nothing here constrains `buf`. This is the shape that let
+// nasa/fprime#5518 through a passing test suite: `procRequest_handler` is
+// ~330 lines and compares `compression_buffer` and `data_reser`, never the
+// port-supplied `fwBuffer`. The bound is computed at run time, which the
+// query must keep accepting for same-object checks -- so only the receiver
+// separates this from `checkedAgainstComputedBound` above.
+void checkOnOtherObject(const Buffer& buf, const Buffer& other) {
+    const FwSizeType limit = requiredSize();
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+    FW_ASSERT(other.getSize() <= limit);
+}
+
+// Violation: the bound is a constant and is strong enough, but it is
+// checked on a different object. Isolates receiver identity from bound
+// arithmetic: a fix that only tightened the bound handling passes the case
+// above and fails this one.
+void checkOnOtherObjectConstantBound(const Buffer& buf, const Buffer& other) {
+    FW_ASSERT(other.getSize() >= Container::MIN_PACKET_SIZE);
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+}
+
+// Violation: the check is on a local copy of a *different* object's size.
+// The receiver has to be carried out through the local-copy form as well as
+// the direct call; a fix that matches receivers only on direct calls still
+// misses this. DpCompressProc has exactly this second, independent path.
+void checkOnLocalCopyOfOtherObject(const Buffer& buf, const Buffer& other) {
+    const FwSizeType otherSize = other.getSize();
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+    FW_ASSERT(otherSize >= Container::MIN_PACKET_SIZE);
+}
+
+// Compliant: `buf` is checked, even though the function also compares a
+// different buffer. One sufficient check is enough -- receiver matching
+// must not degrade into requiring every comparison to be on the subtracted
+// object.
+void checkedAmongOtherObjectChecks(const Buffer& buf, const Buffer& other) {
+    FW_ASSERT(other.getSize() > 0);
+    FW_ASSERT(buf.getSize() >= Container::MIN_PACKET_SIZE);
+    consume(buf.getData(), buf.getSize() - Container::MIN_PACKET_SIZE);
+}

.github/codeql/fprime-queries/FprimeUnguardedUnsignedSubtraction.ql

@@ -0,0 +1,142 @@
+/**
+ * @name Unguarded unsigned subtraction from a run-time size
+ * @description Subtracting a compile-time constant from a size obtained at
+ *              run time underflows to a very large value, rather than going
+ *              negative, when the size is smaller than the constant. A check
+ *              may be a condition or an FW_ASSERT; this reports sites with
+ *              neither.
+ * @kind problem
+ * @id cpp/fprime/unguarded-unsigned-subtraction
+ * @problem.severity warning
+ * @tags correctness
+ *       reliability
+ *       external/cwe/cwe-191
+ */
+
+import cpp
+
+/*
+ * Motivated by nasa/fprime#5518, where `DpCompressProc::procRequest_handler`
+ * computed `fwBuffer.getSize() - Fw::DpContainer::MIN_PACKET_SIZE` on a
+ * port-supplied buffer with no check of any kind in the handler.
+ *
+ * Deliberately conservative, to be usable as a CI gate: the subtrahend must
+ * be a compile-time constant, the minuend must come from a known size
+ * accessor, and check recognition is direction-agnostic with no dominance
+ * analysis. A bound of `C` is accepted for `accessor - K` when `C >= K - 1`,
+ * so `if (size > 0)` credits `size - 1`; `test.cpp` pins that case.
+ *
+ * A check only counts when it reads the size of the *same object* that the
+ * subtraction reads. Without that, any comparison of any object's size
+ * anywhere in the enclosing function suppresses the finding, which is how
+ * the original #5518 defect escaped: `procRequest_handler` is ~330 lines and
+ * compares the sizes of three other buffers, never `fwBuffer`.
+ */
+
+/**
+ * A call returning a size or length determined at run time, rather than
+ * one the calling function chose itself.
+ */
+predicate isSizeAccessor(Call c) {
+  c.getTarget().getName() =
+    [
+      "getSize", "getBuffLength", "getDataSize", "getBuffCapacity",
+      "getDeserializeSizeLeft", "getSerializeSizeLeft"
+    ]
+}
+
+/**
+ * Gets the object that the size accessor call `c` reads from, identified by
+ * the declaration its receiver resolves to: the variable or field accessed,
+ * or the enclosing class when the call is on `this`. Comparing declarations
+ * rather than expressions makes `this->m_buffer.getSize()` in a check match
+ * `this->m_buffer.getSize()` in a subtraction.
+ *
+ * Has no result when the receiver is not one of those shapes, for example
+ * `getBuffer().getSize()`.
+ */
+Declaration sizeAccessorReceiver(Call c) {
+  isSizeAccessor(c) and
+  exists(Expr recv | recv = c.getQualifier() |
+    result = recv.(VariableAccess).getTarget()
+    or
+    recv instanceof ThisExpr and
+    result = c.getTarget().(MemberFunction).getDeclaringType()
+  )
+}
+
+/**
+ * Holds if the size accessor calls `a` and `b` read from the same object.
+ *
+ * When neither receiver resolves to a declaration the calls are treated as
+ * matching. That keeps the previous name-only behaviour for shapes this
+ * query cannot reason about, rather than reporting them.
+ */
+predicate sameReceiver(Call a, Call b) {
+  sizeAccessorReceiver(a) = sizeAccessorReceiver(b)
+  or
+  not exists(sizeAccessorReceiver(a)) and
+  not exists(sizeAccessorReceiver(b)) and
+  isSizeAccessor(a) and
+  isSizeAccessor(b)
+}
+
+/**
+ * Holds if `read` reads the same size, from the same object, as the accessor
+ * call `minuend`: either a direct call, or an access to a local variable
+ * whose initializer is such a call. The local-copy form is idiomatic --
+ * `const FwSizeType n = buf.getSize();` then a check on `n` -- and a check
+ * written that way is still a check; `Fw::DpContainer::setBuffer` does
+ * exactly this.
+ */
+predicate readsSameSize(Expr read, Call minuend) {
+  exists(Call acc |
+    acc = read.(Call)
+    or
+    exists(LocalScopeVariable v |
+      read = v.getAnAccess() and
+      acc = v.getInitializer().getExpr()
+    )
+  |
+    isSizeAccessor(acc) and
+    acc.getTarget().getName() = minuend.getTarget().getName() and
+    sameReceiver(acc, minuend)
+  )
+}
+
+/**
+ * Holds if the function containing `minuend` compares the size that
+ * `minuend` reads -- same accessor, same object -- against a bound that is
+ * not demonstrably too small for `minuend - k`. An `FW_ASSERT` condition
+ * counts: it is an ordinary comparison in the AST, and F Prime treats an
+ * assertion as a valid check -- projects choose their own assert level. A
+ * bound with no constant value is accepted, since the query cannot show it
+ * is too small.
+ */
+bindingset[k]
+predicate hasCheck(Call minuend, float k) {
+  exists(RelationalOperation rop, Expr sizeSide, Expr bound |
+    rop.getEnclosingFunction() = minuend.getEnclosingFunction() and
+    sizeSide = rop.getAnOperand() and
+    bound = rop.getAnOperand() and
+    bound != sizeSide and
+    exists(Expr read | read = sizeSide.getAChild*() and readsSameSize(read, minuend)) and
+    (
+      not exists(bound.getValue())
+      or
+      bound.getValue().toFloat() >= k - 1.0
+    )
+  )
+}
+
+from SubExpr sub, Call minuend, Expr subtrahend, float k
+where
+  sub.getUnspecifiedType().(IntegralType).isUnsigned() and
+  minuend.getParent*() = sub.getLeftOperand() and
+  isSizeAccessor(minuend) and
+  subtrahend = sub.getRightOperand() and
+  k = subtrahend.getValue().toFloat() and
+  not hasCheck(minuend, k)
+select sub,
+  "Unsigned subtraction from " + minuend.getTarget().getName() +
+    "() with no check that it is at least the subtracted constant; this underflows instead of going negative when the size is too small."