[fpc-pascal] Bugs in StrToHostAddr6 in sockets unit

[Michael Van Canneyt]

On Sun, 3 May 2020, Noel Duffy via fpc-pascal wrote:

> On Sun, 3 May 2020 09:57:46 +0200 (CEST) Michael Van Canneyt 
> <michael at freepascal.org> wrote:
>> 
>> On Sun, 3 May 2020, Noel Duffy via fpc-pascal wrote:
>>>
>>> The problem is that the StrToHostAddr6 function doesn't set its return
>>> value until the end of the function. If a parse error occurs
>>> mid-function, it zeroes the record in which it's writing the result but
>>> exits without setting the function return value. What gets returned
>>> depends on what's on the stack.
>>>
>>> I could have sworn that fpc would detect a function exiting without
>>> setting a return value, but clearly 3.0.4 doesn't.
>> 
>> It checks if the result is assigned. This is done;
>> But it does not check this for every exit, as far as I know it never has.
>
> That seems like it would be a valuable feature to have. I am not certain 
> what's contained in an uninitialized variable that gets returned from a 
> function but I'd guess there's a risk of information leakage.

That's why the compiler warns about use of uninitialized variables.
But there will always be corner cases the compiler does not catch.

>>> For the part before the first colon, it has discarded all but the last
>>> two bytes, A2 and F1. But it should not have accepted this string at
>>> all. There can be only four characters between the colons.
>> 
>> If you send a patch for this, I'll apply it.
>
> Sure I can take a stab at this. Do you normally get people to open a bug
>  against which to post the patch? I'm not at all familiar with 
> contributing to fpc, I'm afraid, so please bear with me! Also, if 
> there's information on unit testing for library functions and 
> procedures, that would be very helpful.

Yes, please open a bug report. If you attach a small console test program that
demonstrates the bug (and subsequently the fix) then I will make sure it
ends up in the correct place. If you make sure it exits with exit code 0 if all is
well, and a nonzero exit code if there is an error, that will save me some
work. If you post the URL for the bug here, I will look at it at once.

Thanks for pointing it out!

Michael.