[Bug 3098] remote channel ID seems to be checked with a wrong number

[bugzilla-daemon at bugzilla.mindrot.org]

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

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net

--- Comment #1 from Darren Tucker <dtucker at dtucker.net> ---
(In reply to Takashi Hashida from comment #0)
> https://github.com/openssh/openssh-portable/commit/
> 7ec5cb4d15ed2f2c5c9f5d00e6b361d136fc1e2d#diff-
> 68e5826568dd6f49d090ff4387c220d6R684
> 
> At this commit, remote channel ID upper bound is checked with
> INT_MAX.
> However, it seems that the remote channel ID is uint_32.
> https://tools.ietf.org/html/rfc4254#section-5.1

In the protocol it is, however in the code until this commit it in the
channels struct was an int:
https://github.com/openssh/openssh-portable/commit/9f53229c2

I think what you found is a remnant of some tangled history: the rework
to switch to the new API took a long time and was out-of-tree, so I
suspect the tests you found date back to before the int->uint32_t
change.  The other clue that this is the case is that after the INT_MAX
check rchan is cast to int, which would make sense if remote_id was
still an int:

    c->remote_id = (int)rchan;

As it is I think the test would disallow valid channel ids, can't be
changed into anything sensible (since all 2**32 ids expressible by a
uint32_t are equally valid) and should be deleted.

That still leaves the question of why it's a uint32_t in the header,
but u_int in the function arguments that handle channel ids.

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