[C-safe-secure-studygroup] Meeting Notes from 09 August 2017

[Laurence Urhegyi]

Apologies for the delay in writing these up, but some detailed notes 
from last week's meeting are below, and also added to the wiki of course.

We have two things to discuss in the next meeting before getting into 
the rules themselves:

1) Criteria for rule inclusion for the different profiles.
- Martin has raised the need for a set of criteria for rule inclusion in 
order to create a more objective environment for a vote.
- Especially in regard to rules that have to do with readability. For 
example, Martin has said that Rule 8.13 is generally a good practice to 
follow in any code-base for any software, so should be included in any 
profile. Therefore it seems that if we are going to exclude a rule such 
as that from either profile, we should be clear on why.
- Currently the distinction is somewhat unclear in general, not just 
this rule: although it's a good example because, in summary: this rule 
is good practice, but it is so subjective that it would be totally wrong 
for a project to fail a security audit because of it.

2) Required Visualisation Mechanisms: a potential category of rules 
which inform the programmer what they have done and ask whether it was 
intended.




## Rule 8.13 - Reviewed, no consensus for security profile yet

https://gitlab.com/trustable/C_Safety_and_Security_Rules_Study_Group/wikis/misrarule8.13

Martin: this rule is certainly sound but unlikely to be seen followed in 
practice. It's actually very uncommon to see local pointers declared 
*const* if they aren't being used to modify an object. Seems that 
applying it as strictly as is suggested is not practical. The limited 
scope of the rule makes perfect sense. Would be sensible in both 
profiles, but it should be an advisory rule, as MISRA have outlined it.

Aaron: One thing I am not clear on is that I read this rule as requiring 
all *const* qualifiers on all intermediate pointers as well as the top 
level type, so where you have a pointer that points to another pointer, 
does the first level pointer need to be *const* also?

Whilst I agree with the logic that the final object to be pointed to 
should be *consts* wherever possible, this is not feasible for a 
situation where you have multiple levels of pointers. This will make a 
lot of negative noise. It's rare to see the the pointer type be *const* 
as well as the final object.

Fulvio: that is my understanding of the rule.

Clive: that's how I interpret it also.

Aaron: this rule would need a lot of thinking for a security profile. 
Certainly can't go in in its current form.

**Vote**
Should this rule be included in the Safety Profile?
**Consensus to include this rule in the Safety Profile.**
No consensus on Security Profile.
Aaron: In summary: this rule is good practice, but it is so subjective 
that it would be totally wrong for a project to fail a security audit 
because of it.




## Rule 8.10 - Reviewed, no consensus for security profile yet

https://gitlab.com/trustable/C_Safety_and_Security_Rules_Study_Group/wikis/misrarule8.10

Gavin: If we agree this points to undefined behaviour, we should 
prohibit it.

Martin: The rationale of the rule is not clear to me, the undefined 
behaviour in the first paragraph does not seem specific to inline 
functions, any symbol that declares itself undefined in a program makes 
that program undefined, i don't see why the inline function is specific 
to this problem. Second paragraph talks about the definition of a 
function, and suggests that defining an inline function static has a 
guaranteed impact on the timing of calls to the function, which is not 
quite true. The inline keyword is just a hint to the optimiser to make 
the calls to the function fast as possible. But without defining what 
fast means. 'Fast' depends on not just the function itself but from the 
context which it's called: which could be an inline function, which 
itself could be inline function, etc etc. The compiler can make inline 
decisions regardless of the presence / absence of the inline keyword. 
Cannot see how static storage has any variant on the timing of an 
application or calls to that function. So based on these two points 
alone, I don't think this rule has any merit.

Aaron: Agreed. Looking at the C standard, I agree with the first 
paragraph, this is correct. Second paragraph does not seem to make 
sense: not sure what the benefit is in practice. Not valid from a 
security profile. Not sure about Safety.

Martin: this is not specific to inline functions, declaring external 
'foo' at the top of a translation unit without defining it makes a 
program undefined. So it does not make sense to impose such a rule just 
for inline functions without considering all of the others.

Aaron: the very first paragraph echoes the C standard, so therefore 
there are only two choices presented here: you can use inline 
definitions or external definitions. Compiler needs to pick one but it 
is unspecified which one you get. C standard is explicit about this 
regarding the behaviour.

**Vote**

Should this rule be included in the Safety Profile?

All in favour of including in a Safety profile.

Should this rule be included in the Security Profile?

* 2 votes against
* 1 vote to abstain

We lost Gavin and Martin from this section of the call, so we need to 
re-visit this rule. Need to review this for security.




## Rule 9.3 - Reviewed, no consensus for either profile

https://gitlab.com/trustable/C_Safety_and_Security_Rules_Study_Group/wikis/misrarule9.3

Discussion as per meeting 09 August:

Gavin: this is one where you can have some garbage data, overlay a 
partially initialised array on top of it and then you've planted data 
into that array. This is a similar type of mechanism to a buffer over-run.

Fulvio: with partial initialisation, all the other elements are 
initialised to 0.

Aaron: from a security point of view, exception number 2, where you can 
explicitly do sparse initialisation, seems very strange.

Gavin: yes, this seems to boil down to the programmer knowing what they 
are doing: ie. to use sparse initialisation and not use shortcuts. 
Looking at the examples given in the MISRA rule, it can be unclear where 
things are errors or deliberate. If we take the first example where x[ 3 
] = { 0, 1, 2 }; then we know that's complete initialisation. Whereas on 
the next line, y[ 3 ] { 0, 1 }; is incomplete initialisation, so it is 
unclear if it's an error or is deliberate. Whereas when you get into the 
next example, setting element 1, you have a four element array, and you 
explicitly start setting the second element and then carry on with the 
third element, the implicit initialisation will set it to 0, so it just 
might be an error, rather than being deliberate. So the argument being 
made here, if you look at the compliant example, where 1=1 and 25=2, 
this implies that i can tell 0,2 to 24,26 to the end are excluded.

Aaron: thanks, that makes the logic clearer, this makes more sense now. 
Because there is no way for you to tell the difference between 'I am 
explicitly intending to initialise the next element' as opposed to 'I am 
intending to initialise an element in the sparse array but I have not 
specified which one'.

Clive: I am not convinced by this rule.

Gavin: this is one of those areas where I think the answer here should 
be a requirement that the compiler repeats back to you what you have 
done. So we're dealing with a class of rules that is asking the user 'is 
this what you meant to do?' This question can't be answered by banning 
something from occurring. So there's seems to be a need for explaining 
to the programmer what they have done and whether that was what was 
intended, so they can decide what to do.

Should this rule be included in the Safety profile?

* 2 votes for.
* 2 votes to abstain.

No consensus.

Should this rule be included in the Security profile?

* 2 votes against.
* 1 vote to abstain.
* 1 vote for.


No consensus.

Note that Gavin would like this rule to be included in the Safety 
Profile with the above caveat only, UNLESS we are include a new category 
of 'Required Visualisation Mechanisms' that explain to the programmer 
what they have done, as explained above.




## Rule 9.5 - Reviewed: Consensus to include in both profiles.

https://gitlab.com/trustable/C_Safety_and_Security_Rules_Study_Group/wikis/misrarule9.5


  Discussion during meeting on 09 August:

Should this rule be included in both profiles?

3 votes for, 1 vote against.

Consensus to include in both profiles.