[SSSD] [PATCH] Don't pass user input as a printf format string argument

Stef Walter stefw at redhat.com
Fri Jan 10 13:56:42 UTC 2014


On 10.01.2014 12:11, Jakub Hrozek wrote:
> On Thu, Jan 09, 2014 at 09:32:00PM +0100, Stef Walter wrote:
>> On 08.01.2014 23:27, Jakub Hrozek wrote:
>>> On Wed, Jan 08, 2014 at 09:02:52PM +0100, Stef Walter wrote:
>>>> On 08.01.2014 17:59, Simo Sorce wrote:
>>>>> On Wed, 2014-01-08 at 11:21 +0100, Stef Walter wrote:
>>>>>> On 07.01.2014 22:21, Simo Sorce wrote:
>>>>>>> Sorry I forgot another, I think you should either set errno on errors,
>>>>>>> or return an errno_t instead of -1. Just returning -1 for all errors is
>>>>>>> a poor interface.
>>>>>>
>>>>>> It's the same interface as the functions being replaced: snprintf and
>>>>>> asprintf. We return the full format length, or -1 on failure. snprintf
>>>>>> and friends don't set errno. See man printf(3).
>>>>>>
>>>>>> But I guess now that we're diverging from the interface provided by
>>>>>> snprintf and friends, I don't mind setting errno to EINVAL/ENOMEM if it
>>>>>> makes you happy.
>>>>>
>>>>> Well I generally prefer to know if the format was wrong or ENOMEM
>>>>> happened, but I see you removed the alloc version ... I guess in this
>>>>> case I am not too picky, only format errors can happen now.
>>>>
>>>> I figured that neither neither realmd or sssd was using the asprintf
>>>> variant (both used other allocation schemes) I might as well remove it.
>>>
>>> I agree it's better to not have code that's not used in the repo at all.
>>> Such code can easily become subject to bitrot (it wouldn't be the
>>> case with a utility function like this one perhaps, but still..)
>>
>> One more fix to these patches. I missed a couple signed compare problem
>> problems in patch 0002. fq_len and nlen are now int.
>>
>> Stef
>>
> 
> The patches look good to me and all the testing I did (various formats
> of FQDN including invalid, no FQDN, groups with mixed-domain membership)
> went fine.

Updated version which no longer conflicts with the .gitignore patch.

Stef
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-util-A-safe-printf-for-user-provided-format-strings.patch
Type: text/x-patch
Size: 20111 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140110/47e488f3/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-NSS-Don-t-use-printf-3-on-user-provided-strings.patch
Type: text/x-patch
Size: 25778 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140110/47e488f3/attachment-0001.bin>


More information about the sssd-devel mailing list