wg-best-practices-os-developers
wg-best-practices-os-developers copied to clipboard
Parameter validation checks
Could parameter validation checks be added somewhere in your best practices?
Some background of the area from my own experience.
In the area of safety critical software, ie automotive, aviation, healthcare it's important software never terminates or crashes. Therefore, it's important both callee and the called check their parameters are valid (and then take some appropriate action, be that returning an error, throwing a logic_error, or logging the failure)
So the application when passing an int that must never be negative, must check that, and likewise the API called should also check it is never negative and reject invalid parameters.
Now, here is the big cause of crashes, pointers. Likewise, the application and the API must check the pointer is not NULL. (On APIs that expect valid pointers)
There are issues with compilers using attribute nonnull (an optimization attribute, so compiler can remove NULL checks!), so it can't be advocated for safety critical software.
No amount of soak testing, integration testing, will find all bad unexpected parameters, so we need to take a defensive programming approach. ie think when a user connects a new device to equipment, that wasn't available when the embedded software was developed. That new equipment can't cause any catastrophic failures.
At first I thought you specifically meant the best practices badge. If that was what you meant...
It's an interesting question. One challenge is that the badge is for arbitrary projects, not necessarily safety critical ones or ones that use C. In the specific case of null checks, in many languages those null checks are not merely optimizations but they are enforced by the compilet.
I'm not sure how we could word anything that would cover all those cases. This might be better created as a separate guideline or some such. If you have suggestions on specific wordings I'd be curious. There is of course also MISRA.
If you do not mean just the best practices badge, but a more general question, that sounds like something that maybe would be guidance for highly critical software.
We already recommend using memory safe languages. which prevent the null dereference. Performing null dereference is basically specific to C, C++, and languages that embed them like Objective-C. Rust, Ada, Java, Python, JavaScript, Ruby, and practically all other languages do not dereference nulls by default.
The more general idea of checking parameters is used in many places. However, there are big arguments about whether or not to enable them during production because of overhead.
So it's worthy of discussion, but scoping it and gaining consensus would take time. Evidence, preferably studies of real-world systems, would go a long way if you can point to some.
Thank you for your quick reply David.
I wasn't sure, but I thought maybe you have a guide, a coding standard. In the same way the CppCoreGuidelines have their rules. I saw there are tickets about compiler options for hardening. Perhaps I misunderstood the focus of this project.
I don't recall a MISRA C rule about checking for NULL.
MISRA rule 17.2 in the pointer type conversions section
CERT does have EXP34-C. Do not dereference null pointers
That's good to advocate memory safe languages.
Re the overhead, ALGOL had the ability to disable runtime NULL checks, because back then CPU cycles were limited. Nowadays I don't see an impact on embedded systems checking for NULL.
I don't have any studies, but there are the various standards, ISO 26262 functional safety, DO-178B for aviation etc.
I wasn't sure, but I thought maybe you have a guide, a coding standard. In the same way the CppCoreGuidelines have their rules. I saw there are tickets about compiler options for hardening. Perhaps I misunderstood the focus of this project.
The OpenSSF's scope is anything that can improve the security of open source software, include specifications/standards that can apply to proprietary software too. So this is within possible scope, but we haven't developing any coding guidelines. To be fair, there are already a number of coding guidelines, and some focus on security.
The OpenSSF is currently developing guidelines for compiler flags for C/C++ to improve security, but that's different.
The OpenSSF Best Practices badge has 3 levels: passing, silver, and gold. The passing and gold levels each have a related criterion, but they are only SHOULDs (you must consider them, but you do not need to do them). The problem is that people thought they were a good idea in some circumstances, enough to be worth noting, but that there were so many exceptions to their use that it wasn't worth trying to mandate them for all software. Below are the details.
The OpenSSF Best Practices badge has a passing criterion dynamic_analysis_enable_assertions which is related. It says:
It is SUGGESTED that the project use a configuration for at least some dynamic analysis (such as testing or fuzzing) which enables many assertions. In many cases these assertions should not be enabled in production builds. [dynamic_analysis_enable_assertions] Details: This criterion does not suggest enabling assertions during production; that is entirely up to the project and its users to decide. This criterion's focus is instead to improve fault detection during dynamic analysis before deployment. Enabling assertions in production use is completely different from enabling assertions during dynamic analysis (such as testing). In some cases enabling assertions in production use is extremely unwise (especially in high-integrity components). There are many arguments against enabling assertions in production, e.g., libraries should not crash callers, their presence may cause rejection by app stores, and/or activating an assertion in production may expose private data such as private keys. Beware that in many Linux distributions NDEBUG is not defined, so C/C++ assert() will by default be enabled for production in those environments. It may be important to use a different assertion mechanism or defining NDEBUG for production in those environments. Rationale: Assertions make dynamic analysis more effective, because they increase the number of problems (including vulnerabilities) that dynamic analysis can detect. Other sources also recommend the use of assertions. "Tip #33: If it Can't happen, use assertions to ensure that it won't." ("The Pragmatic Programmer" by Andrew Hunt and David Thomas, p. 122) The paper "Assessing the Relationship between Software Assertions and Code Quality: An Empirical Investigation" by Gunnar Kudrjavets, Nachi Nagappan, and Tom Ball, May 1, 2006, Technical report MSR-TR-2006-54, presented "... an empirical case study of two commercial software components at Microsoft Corporation. The developers of these components systematically employed assertions, which allowed us to investigate the relationship between software assertions and code quality... with an increase in the assertion density in a file there is a statistically significant decrease in fault density. Further, the usage of software assertions in these components found a large percentage of the faults in the bug database."
The OpenSSF Best Practices badge gold level includes the criterion dynamic_analysis_enable_assertions:
The project SHOULD include many run-time assertions in the software it produces and check those assertions during dynamic analysis. {N/A justification} {Met justification} [[dynamic_analysis_enable_assertions](https://bestpractices.coreinfrastructure.org/en/criteria? details=true&rationale=true#2.dynamic_analysis_enable_assertions)]
Thank you for your reply David.
Maybe in the future if you had a coding standard for highly critical software that referred to standards and best practice that would be good.
It's good you cover assert() there is also the newer static_assert(). usually a coding standard would highlight that assert() is for debug builds, that is a developer is running a build without NDEBUG. assert() isn't good for delivered software, as it calls abort(), or in C++ it might call std::terminate() which just raises SIGABRT and core dumps.
Bjarne Stroustrup has a paper on "uncontrolled program termination" in C++ https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2698r0.pdf
For highly critical software, it's better for the software to return an error code (C/C++), or throw a exception (C++) than terminate. (even if we have another watchdog process that re-spawns the application, it may just crash again...)
Maybe other languages are better, eg Rust. Python and others have a lot of implicit type conversions I thought. ADA would be the safest! I'm sure you know all of this, ADA came out of Steelman requirements https://dwheeler.com/steelman/steelman.htm
But Boeing and others have moved from ADA back to C, so we're back to trying to get safe constructs in C.
Our next WG call will be 18July. We'll discuss as a group then.