openssh bugs - May 2020 - [Bug 3167] New: Bugs found by static tests

https://bugzilla.mindrot.org/show_bug.cgi?id=3167

            Bug ID: 3167
           Summary: Bugs found by static tests
           Product: Portable OpenSSH
           Version: 8.2p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: tester7632 at web.de

Running the current version of openssh-portable through SonarQube
revealed a number of bugs.

While there are a lot of false positives or simply unimportant bugs, I
found several areas that seem like legitimate coding errors.


https://sonarcloud.io/project/issues?id=openssh-portable_allfeatures&open=AXIu_cwVij8m0Z5KWuok&resolved=false&types=BUG

https://sonarcloud.io/project/issues?id=openssh-portable_allfeatures&open=AXIu_cwVij8m0Z5KWuop&resolved=false&types=BUG

https://sonarcloud.io/project/issues?id=openssh-portable_allfeatures&open=AXIu_cwxij8m0Z5KWutU&resolved=false&types=BUG

https://sonarcloud.io/project/issues?id=openssh-portable_allfeatures&open=AXIu_c8Tij8m0Z5KWvbw&resolved=false&types=BUG

https://sonarcloud.io/project/issues?id=openssh-portable_allfeatures&open=AXIu_c2tij8m0Z5KWvEw&resolved=false&types=BUG

https://sonarcloud.io/project/issues?id=openssh-portable_allfeatures&open=AXIu_c21ij8m0Z5KWvE9&resolved=false&types=BUG


The complete report can be found here: 
https://sonarcloud.io/project/issues?id=openssh-portable_allfeatures&open=AXIu_c21ij8m0Z5KWvE9&resolved=false&types=BUG


At a first glance I found also issues in the test cases:

https://sonarcloud.io/project/issues?id=openssh-portable-tests&open=AXIu8yLf_34BaxGawOJ2&resolved=false&tags=pitfall&types=BUG

https://sonarcloud.io/project/issues?id=openssh-portable-tests&open=AXIu8yLf_34BaxGawOKT&resolved=false&severities=CRITICAL&types=BUG



I have not gone through all of the reported bugs yet.
The use after free bugs probably need some closer look.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3167

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
(In reply to tester7632 from comment #0)
> https://sonarcloud.io/project/issues?id=openssh-
>
portable_allfeatures&open=AXIu_cwVij8m0Z5KWuok&resolved=false&types=B
> UG
obvious false positive - the static analyser doesn't seem to consider
that an expression may have side-effects.
> 
> https://sonarcloud.io/project/issues?id=openssh-
>
portable_allfeatures&open=AXIu_cwVij8m0Z5KWuop&resolved=false&types=B
> UG
likewise
> https://sonarcloud.io/project/issues?id=openssh-
>
portable_allfeatures&open=AXIu_cwxij8m0Z5KWutU&resolved=false&types=B
> UG
static analyser seems to not understand that variable 'p' is updated
here.
> https://sonarcloud.io/project/issues?id=openssh-
>
portable_allfeatures&open=AXIu_c8Tij8m0Z5KWvbw&resolved=false&types=B
> UG
also false positive. Analyser misses that resolved_len is set around
https://sonarcloud.io/code?id=openssh-portable_allfeatures&selected=openssh-portable_allfeatures%3Asftp-realpath.c&line=99
> https://sonarcloud.io/project/issues?id=openssh-
>
portable_allfeatures&open=AXIu_c2tij8m0Z5KWvEw&resolved=false&types=B
> UG
Another false positive. The path the analyser took demonstrates that
the posited overflow can't occur.
> https://sonarcloud.io/project/issues?id=openssh-
>
portable_allfeatures&open=AXIu_c21ij8m0Z5KWvE9&resolved=false&types=B
> UG
analyser misses the sshbuf_free() call on the very line before the
putative memleak occurs :(
> 
> The complete report can be found here: 
> https://sonarcloud.io/project/issues?id=openssh-
>
portable_allfeatures&open=AXIu_c21ij8m0Z5KWvE9&resolved=false&types=B
> UG
> 
> 
> At a first glance I found also issues in the test cases:
> 
> https://sonarcloud.io/project/issues?id=openssh-portable-
>
tests&open=AXIu8yLf_34BaxGawOJ2&resolved=false&tags=pitfall&types=BUG
> 
> https://sonarcloud.io/project/issues?id=openssh-portable-
>
tests&open=AXIu8yLf_34BaxGawOKT&resolved=false&severities=CRITICAL&ty
> pes=BUG
I think the first one of these at least was fixed in the upstream
version of netcat. I'll sync the OpenSSH copy against that

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3167

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #2 from Damien Miller <djm at mindrot.org>
---
> https://sonarcloud.io/project/issues?id=openssh-portable-
>
tests&open=AXIu8yLf_34BaxGawOKT&resolved=false&severities=CRITICAL&ty
> pes=BUG

This one is another false positive. The analyser misses that the
unix_listener() call is only reachable when family==AF_UNIX and there
there is a check that ensures "host" is not NULL for that case:

https://sonarcloud.io/code?id=openssh-portable-tests&selected=openssh-portable-tests%3Aregress%2Fnetcat.c&line=296

I've synced regress/netcat.c with upstream to fix the duplicate ==-1
check in the other report.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3167

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
close bugs that were resolved in OpenSSH 8.5 release cycle

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.