[Spice-devel] [PATCH spice-xpi] Validate port values

[Peter Hatina]

Hi,

On 04/03/2012 04:40 PM, Marc-André Lureau wrote:
> 
> 
> ----- Mensaje original -----
>> On Tue, Apr 03, 2012 at 03:56:12PM +0200, Peter Hatina wrote:
>>> On 04/03/2012 03:39 PM, Marc-André Lureau wrote:
>>>>
>>>>
>>>> ----- Mensaje original -----
>>>>>  
>>>>>  void nsPluginInstance::Connect()
>>>>>  {
>>>>> +    const int port = portToInt(m_port);
>>>>> +    const int sport = portToInt(m_secure_port);
>>>>> +
>>>>> +    if (port < 0)
>>>>> +        LOG_ERROR("invalid port: " << m_port);
>>>>> +    if (sport < 0)
>>>>> +        LOG_ERROR("invalid secure port: " << m_secure_port);
>>>>> +    if (port < 0 && sport < 0)
>>>>> +        return;
>>>>
>>>> Do we really want to proceed if any of the value is incorrect? I
>>>> would say no.
>>>
>>> You mean, when there is one of the values valid and the other
>>> invalid?
>>> That's what I was asking on #spice. I do not know, if
>>> spicec/virt-viewer
>>> can work only with secure port. With unencrypted port, it works.
>>>
>>> Can anyone competent tell me, if to stop or not?
>>>
>>
>> Marc-Andre, I think it's valid to proceed with a single port as long
>> as
>> the channels are only allocated to the valid port. Besides, -1 is not
>> invalid, it just means "don't open this port".
> 
> Yeah, that's why I would not error out if port is in range [-1..65535], as said in previous patch review: this is not an invalid argument, iiuc.

The function int portToInt(const std::string &port) returns the values
from [-1 to 65535], so you would never error out. -1 was meant to be as
indicator of invalid transport layer port. If both ports are invalid, I
do not want to launch any process, that will do nothing. So the
restriction policy, as far as I am concerned, should be like this.

-- 
Peter Hatina
EMEA ENG-Desktop Development
Red Hat Czech, Brno