IF addrstr == "[" and websocket is true, hostlen becomes 0 and we try
to access addrstr[hostlen-1] which is bad idea.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 ui/vnc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 88f55cbf3c..8830bfe382 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3765,7 +3765,7 @@ static int vnc_display_get_address(const char *addrstr,
 
         addr->type = SOCKET_ADDRESS_TYPE_INET;
         inet = &addr->u.inet;
-        if (addrstr[0] == '[' && addrstr[hostlen - 1] == ']') {
+        if (hostlen >= 2 && addrstr[0] == '[' && addrstr[hostlen - 1] == ']') {
             inet->host = g_strndup(addrstr + 1, hostlen - 2);
         } else {
             inet->host = g_strndup(addrstr, hostlen);
-- 
2.34.1

On 6/12/22 20:23, Vladimir Sementsov-Ogievskiy wrote:
> IF addrstr == "[" and websocket is true, hostlen becomes 0 and we try
> to access addrstr[hostlen-1] which is bad idea.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>   ui/vnc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 88f55cbf3c..8830bfe382 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3765,7 +3765,7 @@ static int vnc_display_get_address(const char *addrstr,
>   
>           addr->type = SOCKET_ADDRESS_TYPE_INET;
>           inet = &addr->u.inet;
> -        if (addrstr[0] == '[' && addrstr[hostlen - 1] == ']') {
> +        if (hostlen >= 2 && addrstr[0] == '[' && addrstr[hostlen - 1] == ']') {
>               inet->host = g_strndup(addrstr + 1, hostlen - 2);
>           } else {
>               inet->host = g_strndup(addrstr, hostlen);

If addrstr is "[" then inet->host ends up being "[" too now, right?

I was pretty sure we had a helper for that, but can't find any.

On 12/6/22 23:12, Philippe Mathieu-Daudé wrote:
> I was pretty sure we had a helper for that, but can't find any.


is uri_parse() from util/uri.c appropriate?

-- 
Best regards,
Vladimir

On 12/6/22 23:12, Philippe Mathieu-Daudé wrote:
> On 6/12/22 20:23, Vladimir Sementsov-Ogievskiy wrote:
>> IF addrstr == "[" and websocket is true, hostlen becomes 0 and we try
>> to access addrstr[hostlen-1] which is bad idea.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   ui/vnc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ui/vnc.c b/ui/vnc.c
>> index 88f55cbf3c..8830bfe382 100644
>> --- a/ui/vnc.c
>> +++ b/ui/vnc.c
>> @@ -3765,7 +3765,7 @@ static int vnc_display_get_address(const char *addrstr,
>>           addr->type = SOCKET_ADDRESS_TYPE_INET;
>>           inet = &addr->u.inet;
>> -        if (addrstr[0] == '[' && addrstr[hostlen - 1] == ']') {
>> +        if (hostlen >= 2 && addrstr[0] == '[' && addrstr[hostlen - 1] == ']') {
>>               inet->host = g_strndup(addrstr + 1, hostlen - 2);
>>           } else {
>>               inet->host = g_strndup(addrstr, hostlen);
> 
> If addrstr is "[" then inet->host ends up being "[" too now, right?
> 
> I was pretty sure we had a helper for that, but can't find any.

that's all a bit strange, let's add a bit of debugging:
diff --git a/ui/vnc.c b/ui/vnc.c
index 88f55cbf3c..b1d463e67a 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3770,6 +3770,7 @@ static int vnc_display_get_address(const char *addrstr,
          } else {
              inet->host = g_strndup(addrstr, hostlen);
          }
+        printf("%s: websocket: %d, host: %s, port: %s\n", __func__, websocket, inet->host, port);
          /* plain VNC port is just an offset, for websocket
           * port is absolute */
          if (websocket) {


then:



./build/qemu-system-x86_64 -vnc [
qemu-system-x86_64: -vnc [: no vnc port specified


./build/qemu-system-x86_64 -vnc [,websocket
qemu-system-x86_64: -vnc [,websocket: warning: short-form boolean option 'websocket' deprecated
Please use websocket=on instead
qemu-system-x86_64: -vnc [,websocket: no vnc port specified



./build/qemu-system-x86_64 -vnc [:0,websocket
qemu-system-x86_64: -vnc [:0,websocket: warning: short-form boolean option 'websocket' deprecated
Please use websocket=on instead
vnc_display_get_address: websocket: 0, host: [, port: 0
vnc_display_get_address: websocket: 1, host: , port: on
qemu-system-x86_64: -vnc [:0,websocket: address resolution failed for [:5900: Name or service not known

./build/qemu-system-x86_64 -vnc [:0,websocket=on
vnc_display_get_address: websocket: 0, host: [, port: 0
vnc_display_get_address: websocket: 1, host: , port: on
qemu-system-x86_64: -vnc [:0,websocket=on: address resolution failed for [:5900: Name or service not known


so, "on" is treated as address string? (aha, that's OK, and it's parsed later in the code)

./build/qemu-system-x86_64 -vnc :0,websocket=[
vnc_display_get_address: websocket: 0, host: , port: 0
we are going to do bad thing!
vnc_display_get_address: websocket: 1, host: , port: [
qemu-system-x86_64: -vnc :0,websocket=[: address resolution failed for :[: Servname not supported for ai_socktype


-- 
Best regards,
Vladimir