From b8b6024bf427bc6986e344638accfc9d1842d3b2 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Mon, 23 Jul 2012 15:02:59 +0100 Subject: [PATCH] SCRAM: correct logic in server-final message handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server-final message looks like this: server-final-message = (server-error / verifier) ["," extensions] server-error = "e=" server-error-value verifier = "v=" base64 The code was trying to check “is there at least one attribute, and is it a verifier?”. But instead it was checking “is there at least one attribute, and if not, is the non-existant attribute a verifier?” by comparing the uninitialized 'attr' variable to 'v'. I've checked other calls to scram_get_next_attr_value() and they seem to get the logic the right way round. This bug does not cause a security vulnerability. If the uninitialized 'attr' variable happens to contain the character 'v', then the following call to scram_check_server_verification() will compare the contents of value (which ought to be a verification string, but is NULL) to the verification string as calculated by Wocky (which is not NULL) and so Wocky will abort the connection. Thanks to Ed Catmur for reporting the use of an uninitialized variable on . --- wocky/wocky-sasl-scram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wocky/wocky-sasl-scram.c b/wocky/wocky-sasl-scram.c index 51c703b..fc28553 100644 --- a/wocky/wocky-sasl-scram.c +++ b/wocky/wocky-sasl-scram.c @@ -530,7 +530,7 @@ scram_handle_server_final_message (WockySaslScram *self, { gchar attr, *value = NULL; - if (!scram_get_next_attr_value (&message, &attr, &value) && attr != 'v') + if (!scram_get_next_attr_value (&message, &attr, &value) || attr != 'v') goto invalid; if (!scram_check_server_verification (self, value)) -- 1.7.10