[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    [oss-security] CVE id request: silc-toolkit
From:       Nico Golde <oss-security+ml () ngolde ! de>
Date:       2009-08-31 16:50:50
Message-ID: 20090831165050.GJ11041 () ngolde ! de
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


Hi,
silc-toolkit upstream fixed [0] various security issues which 
from my assessment allow an attacker arbitrary code 
execution. I'd like to get some CVE ids for these.

|    ASN1: Fix stack variable overwrite when encoding OID.
|
|    The call to sscanf specifies a format string of "%lu", a long unsigned
|    int.  The pointer argument was cast to unsigned long *, but this is
|    wrong for 64 bit systems.  On 64 bit systems, unsigned long is 64 bits,
|    but the oid value is a SilcUInt32 on all systems.  As a result, sscanf
|    will overwrite a neighboring variable on the stack.  Fix this by
|    changing the format string to "%u" and removing the cast.

|    Fixed string format vulnerability in client entry handling.
|
|    Reported and patch provided by William Cummings.

This one allows an attacker to execute arbitrary code, tested.

|     More string format fixes in silcd and client libary

From what I see this is only a problem if full_channel_names settings is used in
SilcClientParams and can't be triggered by an attacker but only by the victim,
maybe I miss something, I'm not that familar with the silc protocol.

|    HTTP: fix stack overwrite due to format string error.
|    
|    On AMD64, %lu refers to a 64-bit unsigned value, but the address passed
|    to sscanf points to a 32-bit unsigned value.  This causes an adjoining
|    value on the stack to be overwritten with data from the converted
|    integer.  Fix the format string to match the size of the supplied value,
|    and remove the pointer cast.

This is only a problem if the internal http server e.g. for checking stats
is enabled.

Can I get CVE ids for the above issues?
The upstream patch is attached.

Cheers
Nico

[0] http://silcnet.org/docs/changelog/SILC%20Toolkit%201.1.10


-- 
Nico Golde - http://www.ngolde.de - nion@jabber.ccc.de - GPG: 0xA0A0AAAA
For security reasons, all text in this mail is double-rot13 encrypted.

["silc.patch" (text/x-diff)]

commit 25a6a61ecf6561bdb00e289175989e28d0fb26bb
Author: kp@valhallalegends.com <kp@valhallalegends.com>
Date:   Sat May 31 16:37:45 2008 -0500

    ASN1: Fix stack variable overwrite when encoding OID.
    
    The call to sscanf specifies a format string of "%lu", a long unsigned
    int.  The pointer argument was cast to unsigned long *, but this is
    wrong for 64 bit systems.  On 64 bit systems, unsigned long is 64 bits,
    but the oid value is a SilcUInt32 on all systems.  As a result, sscanf
    will overwrite a neighboring variable on the stack.  Fix this by
    changing the format string to "%u" and removing the cast.

diff --git a/lib/silcasn1/silcasn1_encode.c b/lib/silcasn1/silcasn1_encode.c
index 8586a8c..3027129 100644
--- a/lib/silcasn1/silcasn1_encode.c
+++ b/lib/silcasn1/silcasn1_encode.c
@@ -351,7 +351,7 @@ silc_asn1_encoder(SilcAsn1 asn1, SilcStack stack1, SilcStack stack2,
 	/* Get OID words from the string */
 	cp = strchr(oidstr, '.');
 	while (cp) {
-	  if (sscanf(oidstr, "%lu", (unsigned long *)&oid) != 1) {
+	  if (sscanf(oidstr, "%u", &oid) != 1) {
 	    SILC_LOG_DEBUG(("Malformed OID string"));
 	    goto fail;
 	  }
@@ -362,7 +362,7 @@ silc_asn1_encoder(SilcAsn1 asn1, SilcStack stack1, SilcStack stack2,
 	  cp = strchr(oidstr, '.');
 
 	  if (!cp) {
-	    if (sscanf(oidstr, "%lu", (unsigned long *)&oid) != 1) {
+	    if (sscanf(oidstr, "%u", &oid) != 1) {
 	      SILC_LOG_DEBUG(("Malformed OID string"));
 	      goto fail;
 	    }

commit 9c93e2c6df752c32bcb64335b418523aae331715
Author: Pekka Riikonen <priikone@silcnet.org>
Date:   Fri Jul 31 22:32:57 2009 +0300

    Fixed string format vulnerability in client entry handling.
    
    Reported and patch provided by William Cummings.

diff --git a/lib/silcclient/client_entry.c b/lib/silcclient/client_entry.c
index 4ec2e0a..6d125b5 100644
--- a/lib/silcclient/client_entry.c
+++ b/lib/silcclient/client_entry.c
@@ -800,10 +800,10 @@ SilcClientEntry silc_client_add_client(SilcClient client,
 		      client_entry->server, sizeof(client_entry->server));
   if (nickname && client->internal->params->full_nicknames)
     silc_snprintf(client_entry->nickname, sizeof(client_entry->nickname),
-		  nickname);
+		  "%s", nickname);
   else if (nickname)
     silc_snprintf(client_entry->nickname, sizeof(client_entry->nickname),
-		  parsed);
+		  "%s", parsed);
 
   silc_parse_userfqdn(username, client_entry->username,
 		      sizeof(client_entry->username),
@@ -890,10 +890,10 @@ void silc_client_update_client(SilcClient client,
 			client_entry->server, sizeof(client_entry->server));
     if (client->internal->params->full_nicknames)
       silc_snprintf(client_entry->nickname, sizeof(client_entry->nickname),
-		    nickname);
+		    "%s", nickname);
     else
       silc_snprintf(client_entry->nickname, sizeof(client_entry->nickname),
-		    parsed);
+		    "%s", parsed);
 
     /* Normalize nickname */
     nick = silc_identifier_check(parsed, strlen(parsed),
@@ -1186,7 +1186,7 @@ SilcClientEntry silc_client_nickname_format(SilcClient client,
         return NULL;
 
       silc_snprintf(client_entry->nickname, sizeof(client_entry->nickname),
-		    cp);
+		    "%s", cp);
       silc_free(cp);
     }
 

commit a785cba501a940921d215c18bc410a53bf1b12e8
Author: Pekka Riikonen <priikone@silcnet.org>
Date:   Fri Aug 7 14:48:46 2009 +0300

    More string format fixes in silcd and client libary

diff --git a/lib/silcclient/command.c b/lib/silcclient/command.c
index a9f9502..71fd348 100644
--- a/lib/silcclient/command.c
+++ b/lib/silcclient/command.c
@@ -955,7 +955,7 @@ SILC_FSM_STATE(silc_client_command_topic)
     }
 
     if (client->internal->params->full_channel_names)
-      silc_snprintf(tmp, sizeof(tmp), conn->current_channel->channel_name);
+      silc_snprintf(tmp, sizeof(tmp), "%s", conn->current_channel->channel_name);
     else
       silc_snprintf(tmp, sizeof(tmp), "%s%s%s",
 		    conn->current_channel->channel_name,
@@ -2143,7 +2143,7 @@ SILC_FSM_STATE(silc_client_command_kick)
     }
 
     if (client->internal->params->full_channel_names)
-      silc_snprintf(tmp, sizeof(tmp), conn->current_channel->channel_name);
+      silc_snprintf(tmp, sizeof(tmp), "%s", conn->current_channel->channel_name);
     else
       silc_snprintf(tmp, sizeof(tmp), "%s%s%s",
 		    conn->current_channel->channel_name,
@@ -2553,7 +2553,7 @@ SILC_FSM_STATE(silc_client_command_leave)
     }
 
     if (client->internal->params->full_channel_names)
-      silc_snprintf(tmp, sizeof(tmp), conn->current_channel->channel_name);
+      silc_snprintf(tmp, sizeof(tmp), "%s", conn->current_channel->channel_name);
     else
       silc_snprintf(tmp, sizeof(tmp), "%s%s%s",
 		    conn->current_channel->channel_name,
@@ -2620,7 +2620,7 @@ SILC_FSM_STATE(silc_client_command_users)
     }
 
     if (conn->client->internal->params->full_channel_names)
-      silc_snprintf(tmp, sizeof(tmp), conn->current_channel->channel_name);
+      silc_snprintf(tmp, sizeof(tmp), "%s", conn->current_channel->channel_name);
     else
       silc_snprintf(tmp, sizeof(tmp), "%s%s%s",
 		    conn->current_channel->channel_name,

commit f9acb085b819a7d0c6b3e9f40bc78f26bc2d429b
Author: kp@valhallalegends.com <kp@valhallalegends.com>
Date:   Fri Dec 12 21:38:54 2008 -0600

    HTTP: fix stack overwrite due to format string error.
    
    On AMD64, %lu refers to a 64-bit unsigned value, but the address passed
    to sscanf points to a 32-bit unsigned value.  This causes an adjoining
    value on the stack to be overwritten with data from the converted
    integer.  Fix the format string to match the size of the supplied value,
    and remove the pointer cast.

diff --git a/lib/silchttp/silchttpserver.c b/lib/silchttp/silchttpserver.c
index fba3ba0..88d6174 100644
--- a/lib/silchttp/silchttpserver.c
+++ b/lib/silchttp/silchttpserver.c
@@ -194,7 +194,7 @@ static SilcBool silc_http_server_parse(SilcHttpServer httpd,
 
     /* Check we have received all data */
     cl = silc_mime_get_field(conn->curheaders, "Content-Length");
-    if (cl && sscanf(cl, "%lu", (unsigned long *)&cll) == 1) {
+    if (cl && sscanf(cl, "%u", &cll) == 1) {
       if (data_len < cll) {
 	/* More data to come */
 	silc_mime_free(conn->curheaders);


[Attachment #6 (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic