openssh bugs - Aug 2005 - [Bug 1066] off-by-one error with GSSAPI names

http://bugzilla.mindrot.org/show_bug.cgi?id=1066

           Summary: off-by-one error with GSSAPI names
           Product: Portable OpenSSH
           Version: 4.1p1
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ssh
        AssignedTo: bitbucket at mindrot.org
        ReportedBy: dleonard at vintela.com


ssh_gssapi_import_name() passes a string through a GSSAPI buffer that is one
byte too long. This seems to occasionally cause problems, like spurious garbage
characters appearing at the end of realms.

------- Additional Comments From dleonard at vintela.com  2005-08-08 23:33
-------
Created an attachment (id=945)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=945&action=view)
quick fix

Simple fix that decrements the buffer length after the snprintf.
A real fix would not be so hacky. :)

------- Additional Comments From djm at mindrot.org  2005-08-27 01:16 -------
Are you saying that gssbuf.value shouldn't be \0 terminated?



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1066





------- Additional Comments From dleonard at vintela.com  2005-08-29 14:51
-------
(In reply to comment #2)
> Are you saying that gssbuf.value shouldn't be \0 terminated?
yes. It's not explicit, but RFC 1509 s2.1.3.2 means gss buffers use a
length,
not a sentinel.





------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1066





------- Additional Comments From simon at sxw.org.uk  2005-08-30 19:24 -------
Yes - all GSSAPI buffers use an explicit length, rather than using NULL as a
marker. This is a bug which should be fixed for correctness's sake.

David's fix is probably as good as any (and leaves the code more readable
than
using a series of memcpy's to avoid the NULL in the first place). Perhaps
the
modification to gssbuf.length should have a comment - to explain that its
stripping the trailing NULL.

However, in the case of import_name, all of the GSSAPI implementations I have
source for then take the buffer, malloc a string 1 character longer, and stick a
NULL back on the end. I can't see anyway in which already having the string
NULL
terminated would cause the problems that David's describing.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1066





------- Additional Comments From dleonard at vintela.com  2005-08-30 20:58
-------
(In reply to comment #4)
> However, in the case of import_name, all of the GSSAPI implementations I
have
> source for then take the buffer, malloc a string 1 character longer, and
stick a
> NULL back on the end. I can't see anyway in which already having the
string NULL
> terminated would cause the problems that David's describing.
You are right; the symptom I am seeing actually came back after this fix, and I
am still trying to track it down. (It was an extra '0' or '@'
after the realm
name, but realm referrals are happening.. it is possibly somewhere beneath the
gssapi interface.)

I am using a gss impl derived from heimdal, and you're right; it does
exactly
the malloc you spoke of. so my analysis was wrong. this is likely a benign bug.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.