PREfast is an older tool by now, but it still gives an nice illustration of
Goals of the project are
You can do the project individually or as a pair. If you do it as a pair, say that you did and mention both your names as comments the code.
You only have to hand in the annotated C++ code, i.e. part I.
Hand this in as a file called "YourName_prefast_exercise.cpp
" with your modified and annotated C++ code,
where YourName
is your full name, without spaces, concatenated.
This is just to ensure that all files have a different name.
If you do the assignment together with someone else, then call
the file "Name1_Name2_prefast_exercise.cpp
" so both your
names are in the the file-name.
Running PREfast, by compiling with the option /analyze
, should
produce 7 warnings: C4996, C6386, C6011, C6217, C6282, C6273, C6031.
If you don't get the C4996 warning, the command line option
/W 3
is probably missing; you have to include that, as in
cl /analyze /W 3 prefast_exercise.cpp
Once that, works follow the steps below:
prefast_exercise.cpp
that PREfast gives, by fixing the code.
Mark places where you changed the code with a comment
//FIXED
to keep track of the changes you made.
Keep the changes to the code minimal; the code is completely silly, no need to completely rewrite it.
_In_count_(...)
if they are only read from;
_Out_cap_(...)
if they are only written from;
_Inout_count_(...)
if they are both read from and written from (but I don't think you need that).
55
or BUF_SIZE
, rather than a program variable, you need another suffix c_
.
So, for example, you can annotate a buffer with
_Out_cap_(len)
or with
_Out_cap_c_(BUF_SIZE)
. (Why this extra c_
is
needed for constants is a mystery to me.)
There is no need to annotate the size of the argument of execute
,
as its size does not really matter. You also do not need to
annotate validate
.
Fix any new warnings this produces.
my_alloc
and do_read
to specify their size, using the annotations
_Ret_cap_(...)
or _Ret_cap_c_(...)
.
Fix any new warnings this produces.
input
to execute
without passing through the validation
operation, and
add calls to the validation routine validate
in the right places
to fix any problems with missing input validation. The steps for
this are explained in more detail below.
To do this, first
input
with [SA_Post(Tainted=SA_Yes)]
, which specifies that this parameter will be tainted as postcondition, and
execute
with
[SA_Pre(Tainted=SA_No)]
to specify the precondition that this parameter should not be tainted.
So you get
HRESULT input([SA_Post(Tainted=SA_Yes)] _Out_... char *buf) {... int execute([SA_Pre(Tainted=SA_No)] _In_ char *buf) {...
Now annotate all the procedures that may handle or produce tainted data using pre- and/or postconditions as above. These procedures are:
do_read
, as it calls input
, which
produces tainted data;
copy_data
, as it is used to copy data coming from
do_read
, which is tainted.
To specify that the return value of a function is tainted, declare it as
[returnvalue:SA_Post(Tainted=SA_Yes)] char* somefunction() { ...PREfast should now produce warnings C6029, when it spots that the program is passing tainted data to the function
execute()
.
validate
in the right places to make such warnings disappear.
As you may notice, PREfast's tainting analysis is not reliable unless you annotate all procedures that may handle tainted data correctly.
execute
and validate
,
for all other functions all parameters and return values
that have a pointer type should have a size annotation
specifing buffer lengths;
Name two advantages and two disadvantages of doing these checks at runtime instead of doing them at compile-time. (I can think of two each. Hint: also think of generic advantages and disadvantages when it comes to runtime vs compile-time checking. Maybe you can think of more?)
zero()
until after you add an annotation about the size of
buf
.
An alternative tool design would be
to produce a warning about zero()
if there are no annotations
for it. (The warning would then not
so much be that there is a potential buffer overflow problem, but
rather that the tool does not have enough information to
determine whether there is a buffer overflow or not.)
Can you give a plausible explanation why PREfast has been designed so
that it does not complain about such unannotated methods?
gets, gets_s, memcpy, printf, system, ...
do, see the online C reference
or C++ reference
specs.
Beware that the syntax of SAL has changed in the past,
so stick to the syntax listed above in the exercises, which is
this SAL version,
as this syntax works with the version of PREfast that we provide.
The new SAL
version 2.0 supported by Visual Studio 2019 uses
more intuitive names: e.g. _In_count_
and
_Out_cap_
are now called _In_reads_
and
_Out_writes_
.
Microsoft has also changed the meaning of the acronym SAL at some stage: it no longer stands for Standard Annotation Language but now stands for Source Code Annotation Language.