Hi,
On 09-11-17 04:25, Antonio Quartulli wrote:
>> @@ -812,6 +816,12 @@ gen_path(const char *directory, const char *filename,
>> struct gc_arena *gc)
>> #else
>> const int CC_PATH_RESERVED = CC_SLASH;
>> #endif
>> +
>> + if (!gc)
>> + {
>> + return NULL; /* Would leak memory otherwise */
>
> As far as I understood the ratio behind this change, we are basically
> saying that passing gc == NULL is a real programming error and not a
> runtime problem. If so, wouldn't it be better to assert(gc) directly?
> I am saying so because we should make this failure louder, so that it
> can be recognized in an early test.
>
> Or can we have cases when this function is called with NULL because of a
> temporary failure (my understanding is that this should not be possible)?I'm a bit in doubt here - if this parameter is NULL, that's a programming error. But we've seen before that programming errors sometimes can be triggered by external input. In this case, this function can be called when a client connects. It *should* never hit this case, but if it somehow does, that becomes a DoS vuln. So I think I prefer 'graceful error handling' in paths that are exposed like this, unless the error indicates a system integrity error such as an out-of-bounds read or write. -Steffan
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
