| Subject: | Improve gss_release_oid() guard against freeing static OIDs |
The GSS-API does not include a gss_release_oid() function; GSS calls
which output OIDs are expected to use OIDs from static storage that do
not need to be freed. Our library includes a gss_release_oid()
function largely to accomodate the gss_str_to_oid() extension.
Providing this function carries the risk that an application might
erroneously call gss_release_oid() with an OID returned by a GSS call,
which would result in freeing static pointers in a naive
implementation. To guard against such bugs (and for historical
reasons not discussed here), gss_release_oid() tries to check whether
the released OID is one of the static OIDs that could have been
returned from a GSS call, and does nothing in that case.
Currently we implement this check by iterating over the mech list
calling gss_internal_release_oid() with the OID pointer. If any mech
returns successfully (indicating that it recognized the OID), we stop
and don't free the OID memory. This approach is brittle: if a
mechanism does not keep its gss_internal_release_oid() implementation
up to date, the guard stops working correctly, but no one notices
until a buggy application calls gss_release_oid() with one of the
particular static OIDs that should have been included in the guard.
Nico suggested a more robust approach: every time a GSS call (other
than gss_str_to_oid()) outputs an OID, we could add that OID pointer
to a global set of guarded OID pointers. Once implemented correctly,
this approach is guaranteed to continue to work for all static OIDs.
A down side is that this mutable set (unless restricted to a fixed
maximum size) would leak each time the GSS library is unloaded on a
platform without library finalizers, but we already have that problem
with the mech table.
(Another, rejected approach is for gss_str_to_oid() to memoize its
results in an ever-growing table, and for gss_release_oid() to do
nothing. This approach could result in a denial of service attack
against an application which calls gss_str_to_oid() based on input
from a possibly untrustworthy party.)
which output OIDs are expected to use OIDs from static storage that do
not need to be freed. Our library includes a gss_release_oid()
function largely to accomodate the gss_str_to_oid() extension.
Providing this function carries the risk that an application might
erroneously call gss_release_oid() with an OID returned by a GSS call,
which would result in freeing static pointers in a naive
implementation. To guard against such bugs (and for historical
reasons not discussed here), gss_release_oid() tries to check whether
the released OID is one of the static OIDs that could have been
returned from a GSS call, and does nothing in that case.
Currently we implement this check by iterating over the mech list
calling gss_internal_release_oid() with the OID pointer. If any mech
returns successfully (indicating that it recognized the OID), we stop
and don't free the OID memory. This approach is brittle: if a
mechanism does not keep its gss_internal_release_oid() implementation
up to date, the guard stops working correctly, but no one notices
until a buggy application calls gss_release_oid() with one of the
particular static OIDs that should have been included in the guard.
Nico suggested a more robust approach: every time a GSS call (other
than gss_str_to_oid()) outputs an OID, we could add that OID pointer
to a global set of guarded OID pointers. Once implemented correctly,
this approach is guaranteed to continue to work for all static OIDs.
A down side is that this mutable set (unless restricted to a fixed
maximum size) would leak each time the GSS library is unloaded on a
platform without library finalizers, but we already have that problem
with the mech table.
(Another, rejected approach is for gss_str_to_oid() to memoize its
results in an ever-growing table, and for gss_release_oid() to do
nothing. This approach could result in a denial of service attack
against an application which calls gss_str_to_oid() based on input
from a possibly untrustworthy party.)