[PATCH 1/7] SECURITY: actually reject messages with improper lengths

Ian Jackson ijackson at chiark.greenend.org.uk
Thu Jun 14 01:38:31 BST 2012


transform_reverse checks to see if the incoming message is a multiple
of the block cipher block size.  If the message fails this check, it
sets *errmsg but it fails to return.  Instead, it proceeds to attempt
to decrypt it.  This will involve a buffer read/write overrun.

It transform_reverse fails to check to see if the incoming message is
long enough to contain the IV, MAC and padding.  This can easily be
exploited to cause secnet to segfault.

buffer_unprepend should check that the buffer has enough data in it to
unprepend.  With the other patches, this should be irrelevant, but it
turns various other potential read overrun bugs into crashes.

site_incoming should check that the incoming message is long enough.
Again, without this, this is an buffer read overrun or possibly a
segfault.

Signed-off-by: Ian Jackson <ijackson at chiark.greenend.org.uk>
---
 site.c      |    3 +++
 transform.c |    5 +++++
 util.c      |    1 +
 3 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/site.c b/site.c
index 624752c..0cd364b 100644
--- a/site.c
+++ b/site.c
@@ -1121,6 +1121,9 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
 			    const struct comm_addr *source)
 {
     struct site *st=sst;
+
+    if (buf->size < 12) return False;
+
     uint32_t dest=ntohl(*(uint32_t *)buf->start);
 
     if (dest==0) {
diff --git a/transform.c b/transform.c
index 8fdf9fd..f55aa44 100644
--- a/transform.c
+++ b/transform.c
@@ -171,6 +171,10 @@ static uint32_t transform_reverse(void *sst, struct buffer_if *buf,
 	return 1;
     }
 
+    if (buf->size < 4 + 16 + 16) {
+	*errmsg="msg too short";
+	return 1;
+    }
 
     /* CBC */
     memset(iv,0,16);
@@ -181,6 +185,7 @@ static uint32_t transform_reverse(void *sst, struct buffer_if *buf,
     /* Assert bufsize is multiple of blocksize */
     if (buf->size&0xf) {
 	*errmsg="msg not multiple of cipher blocksize";
+	return 1;
     }
     serpent_encrypt(&ti->cryptkey,iv,iv);
     for (n=buf->start; n<buf->start+buf->size; n+=16)
diff --git a/util.c b/util.c
index 63fe76f..086c234 100644
--- a/util.c
+++ b/util.c
@@ -269,6 +269,7 @@ void *buf_unappend(struct buffer_if *buf, int32_t amount) {
 
 void *buf_unprepend(struct buffer_if *buf, int32_t amount) {
     void *p;
+    if (buf->size < amount) return 0;
     p=buf->start;
     buf->start+=amount;
     buf->size-=amount;
-- 
1.7.2.5




More information about the sgo-software-discuss mailing list