From c4609169aa7baf208848e72c16d33a3f892353b8 Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Sun, 6 Dec 2015 14:48:19 +0100 Subject: [PATCH] Protect precious strlens, as pointed out by John Goerzen --- citadel/internet_addressing.c | 4 +-- citadel/journaling.c | 30 ++++++++++++++++---- citadel/modules/calendar/serv_calendar.c | 6 ++-- citadel/modules/ctdlproto/serv_messages.c | 2 +- citadel/modules/instmsg/serv_instmsg.c | 2 +- citadel/modules/network/serv_netspool.c | 4 ++- citadel/modules/rssclient/serv_rssclient.c | 31 ++++++++++++--------- citadel/modules/vcard/serv_vcard.c | 24 ++++++++++------ citadel/modules/wiki/serv_wiki.c | 24 ++++++++++++---- citadel/msgbase.c | 32 ++++++++++++---------- 10 files changed, 105 insertions(+), 54 deletions(-) diff --git a/citadel/internet_addressing.c b/citadel/internet_addressing.c index a8868e954..99b22d786 100644 --- a/citadel/internet_addressing.c +++ b/citadel/internet_addressing.c @@ -1227,9 +1227,9 @@ int convert_field(struct CtdlMessage *msg, const char *beg, const char *end) { process_rfc822_addr(value, user, node, name); syslog(LOG_DEBUG, "Converted to <%s@%s> (%s)\n", user, node, name); snprintf(addr, sizeof(addr), "%s@%s", user, node); - if (CM_IsEmpty(msg, eAuthor)) + if (CM_IsEmpty(msg, eAuthor) && !IsEmptyStr(name)) CM_SetField(msg, eAuthor, name, strlen(name)); - if (CM_IsEmpty(msg, erFc822Addr)) + if (CM_IsEmpty(msg, erFc822Addr) && !IsEmptyStr(addr)) CM_SetField(msg, erFc822Addr, addr, strlen(addr)); processed = 1; } diff --git a/citadel/journaling.c b/citadel/journaling.c index 8af9afdca..0d2e74f41 100644 --- a/citadel/journaling.c +++ b/citadel/journaling.c @@ -130,15 +130,33 @@ void JournalRunQueueMsg(struct jnlq *jmsg) { journal_msg->cm_anon_type = MES_NORMAL; journal_msg->cm_format_type = FMT_RFC822; CM_SetField(journal_msg, eJournal, HKEY("is journal")); - CM_SetField(journal_msg, eAuthor, jmsg->from, strlen(jmsg->from)); - CM_SetField(journal_msg, eNodeName, jmsg->node, strlen(jmsg->node)); - CM_SetField(journal_msg, erFc822Addr, jmsg->rfca, strlen(jmsg->rfca)); - CM_SetField(journal_msg, eMsgSubject, jmsg->subj, strlen(jmsg->subj)); + + if (!IsEmptyStr(jmsg->from)) { + CM_SetField(journal_msg, eAuthor, jmsg->from, strlen(jmsg->from)); + } + + if (!IsEmptyStr(jmsg->node)) { + CM_SetField(journal_msg, eNodeName, jmsg->node, strlen(jmsg->node)); + } + + if (!IsEmptyStr(jmsg->rfca)) { + CM_SetField(journal_msg, erFc822Addr, jmsg->rfca, strlen(jmsg->rfca)); + } + + if (!IsEmptyStr(jmsg->subj)) { + CM_SetField(journal_msg, eMsgSubject, jmsg->subj, strlen(jmsg->subj)); + } mblen = snprintf(mime_boundary, sizeof(mime_boundary), "--Citadel-Journal-%08lx-%04x--", time(NULL), ++seq); - rfc822len = strlen(jmsg->rfc822); - + + if (!IsEmptyStr(jmsg->rfc822)) { + rfc822len = strlen(jmsg->rfc822); + } + else { + rfc822len = 0; + } + message_text = NewStrBufPlain(NULL, rfc822len + sizeof(recptypes) + 1024); /* diff --git a/citadel/modules/calendar/serv_calendar.c b/citadel/modules/calendar/serv_calendar.c index 5fb86a228..8fc0dbad1 100644 --- a/citadel/modules/calendar/serv_calendar.c +++ b/citadel/modules/calendar/serv_calendar.c @@ -906,11 +906,11 @@ int ical_conflicts_phase6(struct icaltimetype t1start, existing_msgnum, conflict_event_uid, conflict_event_summary, - ( ((strlen(compare_uid)>0) + ( (!IsEmptyStr(compare_uid) &&(!strcasecmp(compare_uid, conflict_event_uid))) ? 1 : 0 - ) - ); + ) + ); conflict_reported = 1; } diff --git a/citadel/modules/ctdlproto/serv_messages.c b/citadel/modules/ctdlproto/serv_messages.c index 31e48cb8c..0751e28a9 100644 --- a/citadel/modules/ctdlproto/serv_messages.c +++ b/citadel/modules/ctdlproto/serv_messages.c @@ -598,7 +598,7 @@ void cmd_ent0(char *entargs) } free(all_recps); - if ((valid != NULL) && (valid->num_room == 1)) + if ((valid != NULL) && (valid->num_room == 1) && !IsEmptyStr(valid->recp_orgroom)) { /* posting into an ML room? set the envelope from * to the actual mail address so others get a valid diff --git a/citadel/modules/instmsg/serv_instmsg.c b/citadel/modules/instmsg/serv_instmsg.c index 988478537..b150a9167 100644 --- a/citadel/modules/instmsg/serv_instmsg.c +++ b/citadel/modules/instmsg/serv_instmsg.c @@ -243,7 +243,7 @@ int send_instant_message(char *lun, char *lem, char *x_user, char *x_msg) int do_send = 0; /* 1 = send message; 0 = only check for valid recipient */ static int serial_number = 0; /* this keeps messages from getting logged twice */ - if (strlen(x_msg) > 0) { + if (!IsEmptyStr(x_msg)) { do_send = 1; } diff --git a/citadel/modules/network/serv_netspool.c b/citadel/modules/network/serv_netspool.c index eeb35ba71..300efacb3 100644 --- a/citadel/modules/network/serv_netspool.c +++ b/citadel/modules/network/serv_netspool.c @@ -130,7 +130,9 @@ void network_bounce(struct CtdlMessage **pMsg, char *reason) * FIXME ... right now we're just sending a bounce; we really want to * include the text of the bounced message. */ - CM_SetField(msg, eMesageText, reason, strlen(reason)); + if (!IsEmptyStr(reason)) { + CM_SetField(msg, eMesageText, reason, strlen(reason)); + } msg->cm_format_type = 0; /* diff --git a/citadel/modules/rssclient/serv_rssclient.c b/citadel/modules/rssclient/serv_rssclient.c index 697b63b47..17b67317f 100644 --- a/citadel/modules/rssclient/serv_rssclient.c +++ b/citadel/modules/rssclient/serv_rssclient.c @@ -375,20 +375,25 @@ int rss_format_item(AsyncIO *IO, networker_save_message *SaveMsg) StrBufSpaceToBlank(SaveMsg->title); len = StrLength(SaveMsg->title); Sbj = html_to_ascii(ChrPtr(SaveMsg->title), len, 512, 0); - len = strlen(Sbj); - if ((len > 0) && (Sbj[len - 1] == '\n')) - { - len --; - Sbj[len] = '\0'; - } - Encoded = NewStrBufPlain(Sbj, len); - free(Sbj); - - StrBufTrim(Encoded); - StrBufRFC2047encode(&QPEncoded, Encoded); + if (!IsEmptyStr(Sbj)) { + len = strlen(Sbj); + if ((Sbj[len - 1] == '\n')) + { + len --; + Sbj[len] = '\0'; + } + Encoded = NewStrBufPlain(Sbj, len); + - CM_SetAsFieldSB(&SaveMsg->Msg, eMsgSubject, &QPEncoded); - FreeStrBuf(&Encoded); + StrBufTrim(Encoded); + StrBufRFC2047encode(&QPEncoded, Encoded); + + CM_SetAsFieldSB(&SaveMsg->Msg, eMsgSubject, &QPEncoded); + FreeStrBuf(&Encoded); + } + if (Sbj != NULL) { + free(Sbj); + } } if (SaveMsg->link == NULL) SaveMsg->link = NewStrBufPlain(HKEY("")); diff --git a/citadel/modules/vcard/serv_vcard.c b/citadel/modules/vcard/serv_vcard.c index 5c3a3a725..dfd745255 100644 --- a/citadel/modules/vcard/serv_vcard.c +++ b/citadel/modules/vcard/serv_vcard.c @@ -429,7 +429,9 @@ int vcard_upload_beforesave(struct CtdlMessage *msg, recptypes *recp) { CtdlDeleteMessages(CCC->room.QRname, NULL, 0, "[Tt][Ee][Xx][Tt]/.*[Vv][Cc][Aa][Rr][Dd]$"); /* Make the author of the message the name of the user. */ - CM_SetField(msg, eAuthor, usbuf.fullname, strlen(usbuf.fullname)); + if (!IsEmptyStr(usbuf.fullname)) { + CM_SetField(msg, eAuthor, usbuf.fullname, strlen(usbuf.fullname)); + } } /* Insert or replace RFC2739-compliant free/busy URL */ @@ -463,7 +465,7 @@ int vcard_upload_beforesave(struct CtdlMessage *msg, recptypes *recp) { CM_FlushField(msg, eExclusiveID); s = vcard_get_prop(v, "UID", 1, 0, 0); - if (s != NULL) { + if (!IsEmptyStr(s)) { CM_SetField(msg, eExclusiveID, s, strlen(s)); if (CM_IsEmpty(msg, eMsgSubject)) { CM_CopyField(msg, eMsgSubject, eExclusiveID); @@ -477,13 +479,13 @@ int vcard_upload_beforesave(struct CtdlMessage *msg, recptypes *recp) { if (s == NULL) { s = vcard_get_prop(v, "N", 1, 0, 0); } - if (s != NULL) { + if (!IsEmptyStr(s)) { CM_SetField(msg, eMsgSubject, s, strlen(s)); } /* Re-serialize it back into the msg body */ ser = vcard_serialize(v); - if (ser != NULL) { + if (!IsEmptyStr(ser)) { StrBuf *buf; long serlen; @@ -526,7 +528,9 @@ int vcard_upload_aftersave(struct CtdlMessage *msg, recptypes *recp) { /* We're interested in user config rooms only. */ - if ( (strlen(CCC->room.QRname) >= 12) && (!strcasecmp(&CCC->room.QRname[11], USERCONFIGROOM)) ) { + if ( !IsEmptyStr(CCC->room.QRname) && + (strlen(CCC->room.QRname) >= 12) && + (!strcasecmp(&CCC->room.QRname[11], USERCONFIGROOM)) ) { is_UserConf = 1; /* It's someone's config room */ } CtdlMailboxName(roomname, sizeof roomname, &CCC->user, USERCONFIGROOM); @@ -677,7 +681,7 @@ void vcard_write_user(struct ctdluser *u, struct vCard *v) { if (ser == NULL) { ser = strdup("begin:vcard\r\nend:vcard\r\n"); } - if (!ser) return; + if (ser == NULL) return; /* This handy API function does all the work for us. * NOTE: normally we would want to set that last argument to 1, to @@ -938,7 +942,9 @@ void vcard_purge(struct ctdluser *usbuf) { msg->cm_magic = CTDLMESSAGE_MAGIC; msg->cm_anon_type = MES_NORMAL; msg->cm_format_type = 0; - CM_SetField(msg, eAuthor, usbuf->fullname, strlen(usbuf->fullname)); + if (!IsEmptyStr(usbuf->fullname)) { + CM_SetField(msg, eAuthor, usbuf->fullname, strlen(usbuf->fullname)); + } CM_SetField(msg, eOriginalRoom, HKEY(ADDRESS_BOOK_ROOM)); CM_SetField(msg, eNodeName, CtdlGetConfigStr("c_nodename"), strlen(CtdlGetConfigStr("c_nodename"))); CM_SetField(msg, eMesageText, HKEY("Purge this vCard\n")); @@ -1420,7 +1426,9 @@ void store_this_ha(struct addresses_to_be_filed *aptr) { vmsg->cm_format_type = FMT_RFC822; CM_SetField(vmsg, eAuthor, HKEY("Citadel")); s = vcard_get_prop(v, "UID", 1, 0, 0); - CM_SetField(vmsg, eExclusiveID, s, strlen(s)); + if (!IsEmptyStr(s)) { + CM_SetField(vmsg, eExclusiveID, s, strlen(s)); + } ser = vcard_serialize(v); if (ser != NULL) { StrBuf *buf; diff --git a/citadel/modules/wiki/serv_wiki.c b/citadel/modules/wiki/serv_wiki.c index 4642d13d8..7320a156e 100644 --- a/citadel/modules/wiki/serv_wiki.c +++ b/citadel/modules/wiki/serv_wiki.c @@ -222,7 +222,9 @@ int wiki_upload_beforesave(struct CtdlMessage *msg, recptypes *recp) { history_msg->cm_anon_type = MES_NORMAL; history_msg->cm_format_type = FMT_RFC822; CM_SetField(history_msg, eAuthor, HKEY("Citadel")); - CM_SetField(history_msg, eRecipient, CCC->room.QRname, strlen(CCC->room.QRname)); + if (!IsEmptyStr(CCC->room.QRname)){ + CM_SetField(history_msg, eRecipient, CCC->room.QRname, strlen(CCC->room.QRname)); + } CM_SetField(history_msg, eExclusiveID, history_page, history_page_len); CM_SetField(history_msg, eMsgSubject, history_page, history_page_len); CM_SetField(history_msg, eSuppressIdx, HKEY("1")); /* suppress full text indexing */ @@ -640,11 +642,23 @@ void wiki_rev(char *pagename, char *rev, char *operation) } else if (!strcasecmp(operation, "revert")) { CM_SetFieldLONG(msg, eTimestamp, time(NULL)); - CM_SetField(msg, eAuthor, CCC->user.fullname, strlen(CCC->user.fullname)); - CM_SetField(msg, erFc822Addr, CCC->cs_inet_email, strlen(CCC->cs_inet_email)); - CM_SetField(msg, eOriginalRoom, CCC->room.QRname, strlen(CCC->room.QRname)); + if (!IsEmptyStr(CCC->user.fullname)) { + CM_SetField(msg, eAuthor, CCC->user.fullname, strlen(CCC->user.fullname)); + } + + if (!IsEmptyStr(CCC->cs_inet_email)) { + CM_SetField(msg, erFc822Addr, CCC->cs_inet_email, strlen(CCC->cs_inet_email)); + } + + if (!IsEmptyStr(CCC->room.QRname)) { + CM_SetField(msg, eOriginalRoom, CCC->room.QRname, strlen(CCC->room.QRname)); + } + CM_SetField(msg, eNodeName, CtdlGetConfigStr("c_nodename"), strlen(CtdlGetConfigStr("c_nodename"))); - CM_SetField(msg, eExclusiveID, pagename, strlen(pagename)); + + if (!IsEmptyStr(pagename)) { + CM_SetField(msg, eExclusiveID, pagename, strlen(pagename)); + } msgnum = CtdlSubmitMsg(msg, NULL, "", 0); /* Replace the current revision */ } else { diff --git a/citadel/msgbase.c b/citadel/msgbase.c index c65eef1a6..49f634595 100644 --- a/citadel/msgbase.c +++ b/citadel/msgbase.c @@ -2832,7 +2832,7 @@ long CtdlSubmitMsg(struct CtdlMessage *msg, /* message to save */ /* * If this message has no O (room) field, generate one. */ - if (CM_IsEmpty(msg, eOriginalRoom)) { + if (CM_IsEmpty(msg, eOriginalRoom) && !IsEmptyStr(CCC->room.QRname)) { CM_SetField(msg, eOriginalRoom, CCC->room.QRname, strlen(CCC->room.QRname)); } @@ -3055,10 +3055,10 @@ void quickie_message(const char *from, msg->cm_anon_type = MES_NORMAL; msg->cm_format_type = format_type; - if (from != NULL) { + if (!IsEmptyStr(from)) { CM_SetField(msg, eAuthor, from, strlen(from)); } - else if (fromaddr != NULL) { + else if (!IsEmptyStr(fromaddr)) { char *pAt; CM_SetField(msg, eAuthor, fromaddr, strlen(fromaddr)); pAt = strchr(msg->cm_fields[eAuthor], '@'); @@ -3070,17 +3070,19 @@ void quickie_message(const char *from, msg->cm_fields[eAuthor] = strdup("Citadel"); } - if (fromaddr != NULL) CM_SetField(msg, erFc822Addr, fromaddr, strlen(fromaddr)); - if (room != NULL) CM_SetField(msg, eOriginalRoom, room, strlen(room)); + if (!IsEmptyStr(fromaddr)) CM_SetField(msg, erFc822Addr, fromaddr, strlen(fromaddr)); + if (!IsEmptyStr(room)) CM_SetField(msg, eOriginalRoom, room, strlen(room)); CM_SetField(msg, eNodeName, CtdlGetConfigStr("c_nodename"), strlen(CtdlGetConfigStr("c_nodename"))); - if (to != NULL) { + if (!IsEmptyStr(to)) { CM_SetField(msg, eRecipient, to, strlen(to)); recp = validate_recipients(to, NULL, 0); } - if (subject != NULL) { + if (!IsEmptyStr(subject)) { CM_SetField(msg, eMsgSubject, subject, strlen(subject)); } - CM_SetField(msg, eMesageText, text, strlen(text)); + if (!IsEmptyStr(text)) { + CM_SetField(msg, eMesageText, text, strlen(text)); + } CtdlSubmitMsg(msg, recp, room, 0); CM_Free(msg); @@ -3520,7 +3522,7 @@ struct CtdlMessage *CtdlMakeMessageLen( if (myelen > 0) { CM_SetField(msg, eMessagePath, my_email, myelen); } - else { + else if (!IsEmptyStr(author->fullname)) { CM_SetField(msg, eMessagePath, author->fullname, strlen(author->fullname)); } convert_spaces_to_underscores(msg->cm_fields[eMessagePath]); @@ -3538,11 +3540,13 @@ struct CtdlMessage *CtdlMakeMessageLen( CM_SetAsFieldSB(msg, eAuthor, &FakeEncAuthor); FreeStrBuf(&FakeAuthor); - if (CCC->room.QRflags & QR_MAILBOX) { /* room */ - CM_SetField(msg, eOriginalRoom, &CCC->room.QRname[11], strlen(&CCC->room.QRname[11])); - } - else { - CM_SetField(msg, eOriginalRoom, CCC->room.QRname, strlen(CCC->room.QRname)); + if (!!IsEmptyStr(CCC->room.QRname)) { + if (CCC->room.QRflags & QR_MAILBOX) { /* room */ + CM_SetField(msg, eOriginalRoom, &CCC->room.QRname[11], strlen(&CCC->room.QRname[11])); + } + else { + CM_SetField(msg, eOriginalRoom, CCC->room.QRname, strlen(CCC->room.QRname)); + } } CM_SetField(msg, eNodeName, CtdlGetConfigStr("c_nodename"), strlen(CtdlGetConfigStr("c_nodename"))); -- 2.30.2