From 619936a51adeb81a850ddc51832fff4f10ee97b9 Mon Sep 17 00:00:00 2001 From: Jim Norman Date: Wed, 26 Apr 2006 15:10:02 +0000 Subject: [PATCH] Security Audit 5.13: Ensure that string lengths are within limits and null terminated before copying them to buffers. --- CASA.changes | 5 +++ c_adlib/ad_gk/GnomeKeyring.cs | 5 +-- c_adlib/ad_gk/native/ad_gk.c | 58 +++++++++++++++++++++++++++++------ 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/CASA.changes b/CASA.changes index d5ba3929..c4d2f764 100644 --- a/CASA.changes +++ b/CASA.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Wed Apr 26 09:10:20 MST 2006 - jnorman@novell.com +- Security Audit 5.13: Ensure that string lengths are within limits + and null terminated before copying them to buffers. + ------------------------------------------------------------------- Wed Apr 26 12:53:10 IST 2006 - smanojna@novell.com - Bug 165283: CASA docs and About screen states that CASA runs on diff --git a/c_adlib/ad_gk/GnomeKeyring.cs b/c_adlib/ad_gk/GnomeKeyring.cs index 5577f344..495f067a 100644 --- a/c_adlib/ad_gk/GnomeKeyring.cs +++ b/c_adlib/ad_gk/GnomeKeyring.cs @@ -50,8 +50,9 @@ namespace Novell.CASA.DataEngines.GK public int cTime; public NativeItemInfo() { - displayName = Marshal.AllocHGlobal(128); - secret = Marshal.AllocHGlobal(128); + /* The GUI allows 256 as the max number of chars for these items */ + displayName = Marshal.AllocHGlobal(256); + secret = Marshal.AllocHGlobal(256); } ~NativeItemInfo() { diff --git a/c_adlib/ad_gk/native/ad_gk.c b/c_adlib/ad_gk/native/ad_gk.c index 27e7a92c..6ae7b528 100644 --- a/c_adlib/ad_gk/native/ad_gk.c +++ b/c_adlib/ad_gk/native/ad_gk.c @@ -740,15 +740,38 @@ void ItemGetInfoCb( GnomeKeyringResult result, { GetItemInfoCbData *cbData = data; ItemInfo *itemInfo = cbData->info; + char *item; + size_t itemlen = 0, maxlen = 0; if (result != GNOME_KEYRING_RESULT_OK) { //g_print ("Unable to get Item info: %d\n", result); } else { - itemInfo->itemType = gnome_keyring_item_info_get_type(info); - strcpy(itemInfo->displayName,gnome_keyring_item_info_get_display_name(info)); - strcpy(itemInfo->secret,gnome_keyring_item_info_get_secret(info)); + /* maxlen = 255. This should be one less than the size of + Novell.CASA.DataEngines.GK.NativeItemInfo.displayName */ + maxlen = sizeof (itemInfo->displayName); + item = gnome_keyring_item_info_get_display_name(info); + itemlen = strlen (item); + if (itemlen > maxlen) { + itemInfo->displayName = NULL; + } else { + strncpy(itemInfo->displayName, item, maxlen); + itemInfo->displayName[maxlen] = '\0'; + } + + /* maxlen = 255. This should be one less than the size of + Novell.CASA.DataEngines.GK.NativeItemInfo.secret */ + maxlen = sizeof (itemInfo->secret); + item = gnome_keyring_item_info_get_secret(info); + itemlen = strlen (item); + if (itemlen > maxlen) { + itemInfo->secret = NULL; + } else { + strncpy(itemInfo->secret, item, maxlen); + itemInfo->secret[maxlen] = '\0'; + } + itemInfo->mTime = gnome_keyring_item_info_get_mtime(info); itemInfo->cTime = gnome_keyring_item_info_get_ctime(info); @@ -765,6 +788,7 @@ void ItemGetAttributesCb(GnomeKeyringResult result, GetAttributeListCbData *cbData = data; Attribute *attr = NULL; + size_t attrlen = 0; GList **retList; retList = cbData->attrList; @@ -788,11 +812,21 @@ void ItemGetAttributesCb(GnomeKeyringResult result, memset(attr,0,sizeof(Attribute)); attr->type = 0; attr->key = (char*)malloc(KEY_SIZE); - if(attr->key != NULL) - strcpy(attr->key,attrList[i].name); + if(attr->key != NULL) { + attrlen = strlen (attrList[i].name); + if (attrlen > (KEY_SIZE - 1)) + attrlen = KEY_SIZE - 1; + strncpy(attr->key, attrList[i].name, attrlen); + attr->key[attrlen] = '\0'; + } attr->value = (char*)malloc(VAL_SIZE); - if(attr->value != NULL) - strcpy(attr->value,attrList[i].value.string); + if(attr->value != NULL) { + attrlen = strlen (attrList[i].value.string); + if (attrlen > (VAL_SIZE - 1)) + attrlen = VAL_SIZE - 1; + strncpy(attr->value, attrList[i].value.string, attrlen); + attr->value[attrlen] = '\0'; + } } } else if(attrList[i].type == GNOME_KEYRING_ATTRIBUTE_TYPE_UINT32) @@ -803,8 +837,14 @@ void ItemGetAttributesCb(GnomeKeyringResult result, memset(attr,0,sizeof(Attribute)); attr->type = 0; attr->key = (char*)malloc(KEY_SIZE); - if(attr->key != NULL) - strcpy(attr->key,attrList[i].name); + if(attr->key != NULL) { + attrlen = strlen (attrList[i].name); + /* We simply truncate the name if it is greater than KEY_SIZE */ + if (attrlen > (KEY_SIZE - 1)) + attrlen = KEY_SIZE - 1; + strncpy(attr->key, attrList[i].name, attrlen); + attr->key [attrlen] = '\0'; + } attr->value = (char*)malloc(VAL_SIZE); if(attr->value != NULL) sprintf(attr->value,"%d",attrList[i].value.integer);