login to google talk with wrong username/password leads to segmentation fault

Reported by buzzdee on 2009-10-25
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Coccinella
Medium
Unassigned

Bug Description

On OpenBSD, using Coccinella source from SVN I run into a segfault when trying to connect to google talk with the default google talk account. It seems a "forbidden" is sent, instead of "bad-auth" or "not-authorized".

appended patch fixes the problem for me.

buzzdee (sebastia) wrote :
sander (s-devrieze) wrote :

According to http://xmpp.org/rfcs/rfc3920.html#sasl (section 6.4), the correct server response would be "<not-authorized/>". "<forbidden/>" is no valid response. As it is important that anyone can create a full-featured XMPP client by only using the official open protocols, it is *forbidden* to apply this patch to Coccinella.

Of course, Coccinella should not segfault (it doesn't do this on Linux btw). So, instead, I would suggest to rewrite the code to use a switch command. If the value is not found, this following string should be showed:

"Invalid error. Bug in server software: error <%s/> not defined in section 6.4 SASL Errors of RFC 3920. Please consider reporting this bug to your server administrator."

buzzdee (sebastia) wrote :

"Invalid error. Bug in server software: error <%s/> not defined in section 6.4 SASL Errors of RFC 3920. Please consider reporting this bug to your server administrator."

Just popping up this error would be even easier to implement I think.
I'll do as you suggested.

However, also in the Login.tcl I found this comment:
                # Added 'bad-auth' which seems to be a ejabberd anachronism.
                set errcode [string map {bad-auth not-authorized} $errcode]

so it is mapping bad-auth to not-authorized.

According to the RFC you referenced, bad-auth is A) also not mentioned there, and B) my ejabberd server sends not-authorized. So I guess this bad-auth was for older versions of ejabberd?
I'd say this mapping should also be removed, what do you think?

buzzdee (sebastia) wrote :
Download full text (4.1 KiB)

Forgot, the segfault is probably due to the version of tcl/tk I have running on OpenBSD. On Linux I get the following exception:

key "forbidden" not known in dictionary
key "forbidden" not known in dictionary
    while executing
"dict get $xmppShort $errcode"
    (procedure "GetErrorStr" line 64)
    invoked from within
"GetErrorStr $errcode $errmsg"
    (procedure "HandleErrorCode" line 11)
    invoked from within
"HandleErrorCode $errcode $errmsg"
    (procedure "HighFinal" line 24)
    invoked from within
"HighFinal $token $jlibname $status $errcode $errmsg"
    (procedure "::Login::HighLoginCB" line 11)
    invoked from within
"::Login::HighLoginCB ::Login::high3 ::jlib::jlib1 error forbidden {Username or password not correct.}"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 $state(-command) [list $jlibname $status $errcode $errmsg]"
    (procedure "finish" line 49)
    invoked from within
"finish $jlibname $errcode $errmsg"
    (procedure "auth_cb" line 10)
    invoked from within
"auth_cb ::jlib::jlib1 error {forbidden {Username or password not correct.}}"
    (in namespace inscope "::jlib::connect" script line 1)
    invoked from within
"::namespace inscope ::jlib::connect auth_cb ::jlib::jlib1 error {forbidden {Username or password not correct.}}"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 $cmd [list $jlibname $type $subiq]"
    (procedure "::jlib::invoke_iq_callback" line 5)
    invoked from within
"::jlib::invoke_iq_callback ::jlib::jlib1 {::namespace inscope ::jlib::connect auth_cb} error {forbidden {Username or password not correct.}}"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 $iqcmd($id) [list error $errspec]"
    (procedure "iq_handler" line 83)
    invoked from within
"iq_handler $jlibname $xmldata"
    (procedure "::jlib::dispatcher" line 9)
    invoked from within
"::jlib::dispatcher ::jlib::jlib1 {iq {id 1038 type error} 0 {} {{query {xmlns jabber:iq:auth} 0 {} {{username {} 0 You {}} {resource {} 0 Coccinella@s..."
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 $wrapper($id,parsecmd) [list $wrapper($id,tree,2)]"
    (procedure "::wrapper::elementend" line 41)
    invoked from within
"::wrapper::elementend wrap1 iq -namespace urn:ietf:params:xml:ns:xmpp-stanzas"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 $options(-elementendcommand) [list $tag] $empty $ns"
    invoked from within
"ParseEvent:ElementClose $tag [array get options]"
    invoked from within
"::sgml::parseEvent {{} {} {} {} iq {} { type="error" id="1038"} {} query {} { xmlns="jabber:iq:auth"} {} username {} {} You username / {} {} resource ..."
    ("eval" body line 1)
    invoked from within
"eval ::sgml::parseEvent [list $tokenised] $parseOptions"
    (procedure "parse" line 60)
    invoked from within
"parse xmlparser2 {<iq type="error" id="1038"><query xmlns="jabber:iq:auth"><username>You</username><resource>Coccinella@sre</resource><password/></que..."
    (in namespace inscope "::xml::tclparser" script line 1)
    invoked from within
"::namespace inscope ::xml::tclparser parse xmlparser2 {<iq type="error" id="1038"><query xmlns="jabber:iq:auth"><username>You</...

Read more...

sander (s-devrieze) wrote :

Yes, the bad-auth code also should be removed/replaced by similar code.

buzzdee (sebastia) wrote :

appended patch catches all unknown responses, and presents the user with an error message.

also the mentioned bad-auth is now catched by the error message.

buzzdee (sebastia) wrote :

 I would change the text to "Invalid server response due to bug in server software: error <%s/> is not defined in section 6.4 SASL Errors of RFC 3920. Please consider reporting this bug to your server administrator." Also the errmsg maybe can be leaved out for this error message?

buzzdee (sebastia) wrote :

svn revision 2750 should fix the issue.
Also the mapping for bad-auth was removed.
The error message in the end is not left out, as, with the actual code flow, this would mean, more code to handle that, which would be counterproductive to comment #2 and #3.

Changed in coccinella:
status: New → Fix Committed
buzzdee (sebastia) on 2009-12-03
Changed in coccinella:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers