[Swan-dev] integer types biting us [long]
D. Hugh Redelmeier
hugh at mimosa.com
Thu Aug 11 18:38:42 EEST 2022
1)
intmax_t and uintmax_t should never be used.
Those types were always bit sloppy. For example, they may be quite
inefficient.
They used to be safe to use in code that was trying to be generic. A
function parameter of this type could handle any int type thrown at it (if
the signedness was right). A macro could define a temporary that could
handle any int type thrown at it (but the typeof extension was better).
The *intmax_t types were almost never a good idea for the result of a
function unless the receiver expected that type. A nice exception is
illustrated by a max function: the value of the result is bounded by the
values of its arguments.
But the C Standards committee has decided that preserving old ABIs is more
important than preserving the standard's promise to programmers.
Some ABIs (note: not produced by the Committee) use *intmax_t. The
Committee wants to allow new, wider int types. It does not want to break
those ABIs, so it is breaking *intmax_t: new wider integer types can be
wider than *intmax_t.
This is a Bad Thing:
- the only point of the *intmax_t types was to create a universal integer
type. This point disappears.
- this promise to the C programmer has been rescinded.
- The is a silent change: no compile-time error message can be generated.
- there is explicitly a promise that C changes will not break programs and
equally explicitly the Committee has reserved the right to break
implementation
This change breaks the C Standards Committee's explicit remit.
I told the Committee so but they did this anyway.
breaks their explicit remit. I think that this is a scandal, but they
didn't listen to me.
*intmax_t no longer is promised to be able to encompass all
integral types of the appropriate signedness.
So *intmax_t has lost all its purpose.
The Committee should have removed it from the language rather than
silently breaking code that used it.
The ABIs that cannot change binaries should change declarations to use the
appropriate explict types. This is an easy change for ABIs.
2)
Andrew mentioned https://github.com/libreswan/libreswan/issues/763
[12:37] <cagney> addconn overflows uintmax_t replay-window=4294967294 to
replay-window=18446744073709551614 #763
[12:46] <cagney>
testing/pluto/pfkeyv2-replay-window-freebsd/freebsdw.console.txt
[12:48] <cagney> replay-window=4294967294 # UINT32_MAX - 1
4294967294 is (uint32) -2
18446744073709551614 is (uint64_t) -1
This demonstrates more than one bug.
3)
[Exploring a bit]
In libipsecconf the universal integer type is uintmax_t
include/ipsecconf/keywords.h:
struct kw_list {
struct kw_list *next;
struct keyword keyword;
char *string;
/**/ uintmax_t number;
};
But also plain "int" (i.e. signed).
include/ipsecconf/confread.h:
typedef int knf[KEY_NUMERIC_ROOF];
struct starter_config {
struct {
ksf strings;
knf options;
str_set strings_set;
int_set options_set;
} setup;
/* conn %default */
struct starter_conn conn_default;
char *ctlsocket; /* location of pluto control socket */
/* connections list (without %default) */
TAILQ_HEAD(, starter_conn) conns;
};
There are cases of a knf member being assigned a kw_list number
without bounds checking. There are cases of a knf member being
assigned to an unsigned object without bounds checking. I have not
done a census to count the times.
How many options are actually signed? (I.e. can we eliminated signed
integral types?)
How many (signed or unsigned) integer options cannot be limited to
INTMAX_T? (I.e. can we use an signed type as a universal option
holder?)
Maybe we cannot have a universal integral option holder.
4)
Every time we narrow an integral value to assign it to an object with an
integer type, we should check if it fits.
This is a special case of overflow checking. We should always do overflow
checking.
This is tiresome busywork. But this is major source of bugs and in
particular security bugs. Especially in C code.
When I code in C, I'm constantly proving to myself that overflow cannot
happen. Sometimes I slip up. Sometimes the proof is tricky and should
appear in comments (tiresome busywork).
We should add helper functions so that it is less annoying to add the
required checks.
It would be nice if the C compiler could flag suspicious narrowing at
compile time. Unfortunately, that would probably be unacceptably
noisy.
4)
Retrofitting overflow checking is typically a lot of tricky work. But
it needs doing. It can have security implications.
More information about the Swan-dev
mailing list