Package: licq
Version: 1.3.0-4
Severity: important

Hi,

I've just stumbled across a buffer overflow in
icqd.cpp:CICQDaemon::SaveConf() (line 692 in
the pristine file). When the owner list is
written,

    772   int n = 1;
    773   FOR_EACH_OWNER_START(LOCK_R)
    774   {
    775     char szOwnerId[11], szOwnerPPID[11];
    776     char szPPID[5];
    777     sprintf(szOwnerId, "Owner%d.Id", n);
    778     sprintf(szOwnerPPID, "Owner%d.PPID", n++);
    779 
    780     szPPID[0] = (pOwner->PPID() & 0xFF000000) >> 24;
    781     szPPID[1] = (pOwner->PPID() & 0x00FF0000) >> 16;
    782     szPPID[2] = (pOwner->PPID() & 0x0000FF00) >> 8;
    783     szPPID[3] = (pOwner->PPID() & 0x000000FF);
    784     szPPID[4] = '\0';
    785 
    786     pOwner->SaveLicqInfo();
    787     if (strcmp(pOwner->IdString(), "0") != 0)
    788     {
    789       licqConf.WriteStr(szOwnerId, pOwner->IdString());
    790       licqConf.WriteStr(szOwnerPPID, szPPID);
    791     }
    792   }
    793   FOR_EACH_OWNER_END

szOwnerPPID gets at least 12 bytes (!) sprintf()'ed to it
(including the closing \0), depending on the value of n.
As the memory layout happens to place szOwnerId directly
after szOwnerPPID, the former's first byte is overwritten,
which finally results in (line 789)

licqConf.writeStr("\0...", "<my OwnerID>");

and produces a licq.conf like this:

[owners]
NumOfOwners = 1
Owner1.Id = 0
Owner1.PPID = Licq
 = <my OwnerID>


(The outcome is that upon each subsequent startup of licq,
I have to reset the UIN via the Owner Manager to its real
value, as it's always initialized to "0")

Doing dynamic allocation based on ol->size() should fix it
(as would increasing the buffer size above a reasonable
length maximum of ol->size()).


Thank you,

Jan

-- 
Jan C. Nordholz
<jckn At gmx net>

Attachment: signature.asc
Description: Digital signature

Reply via email to