[Bug 2055] New: channel_setup_fwd_listener return value used incorrectly

[bugzilla-daemon at mindrot.org]

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

            Bug ID: 2055
           Summary: channel_setup_fwd_listener return value used
                    incorrectly
    Classification: Unclassified
           Product: Portable OpenSSH
           Version: 6.1p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: mathieu.lacage at gmail.com

It looks like the code for channel_setup_fwd_listener is written to
return 1 on success, and 0 on failure. The user of this function from
mux.c (indirectly thru channel_setup_local_fwd_listener) appears to be
assuming a negative value on failure and 0 on success (the standard
unix semantics)

i.e., none of the users of this function report port allocation errors
correctly which means that it is not possible to reliably detect port
allocation failure when setting up port allocations thru ssh -S /tmp/x
O forward. i.e. it sucks :/

How to reproduce:

ssh -L1040:localhost:1000  localhost &
ssh -M -S /tmp/x -NT localhost &
ssh -S /tmp/x -L1040:localhost:1000 -O forward localhost

What should happen:
from reading the source code in mux.c, I believe that the last command
should print something along the lines of: "Master refused forwarding
request: Port forwarding failed"

What happens:
no output from last command that appears to succeed.

I suspect that somewhere along the lifetime of this code, the callee
was changed to return its success/failure differently but the callers
were not updated.

The fix is fairly obvious (although I did not bother to recompile/test
it): 

in mux.c, change 
                if (channel_setup_local_fwd_listener(fwd.listen_host,
                    fwd.listen_port, fwd.connect_host,
fwd.connect_port,
                    options.gateway_ports) < 0) {

to:
                if (channel_setup_local_fwd_listener(fwd.listen_host,
                    fwd.listen_port, fwd.connect_host,
fwd.connect_port,
                    options.gateway_ports) == 0) {

Note: from looking at the code, I believe that -R is immune to this
problem. -D should suffer from the same problem although I did not test
it.

Enjoy!

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