Toolkit object ID and comment retrieval functions are not safe

[LRossman]

Toolkit functions such as EN_getnodeid, EN_getlinkid, etc., and EN_getcomment are potentially unsafe because they use strcpy to copy an object's ID or comment into an output string passed into the function. Although the Toolkit documentation states that the output string should be sized by the client to EN_MAXID+1 for ID names (or EN_MAXMSG+1 for comments) there is no way for the retrieval functions to check for this. If an output string is sized smaller than the string being retrieved a buffer overrun will occur and memory in the client's application will get overwritten with the potential to cause serious problems.

A fix for safely retrieving ID names is as follows:

In epanet2_enums.h add the following definition:

typedef char EN_ID[EN_MAXID+1]; 

and add the following new function declaration to epanet2_2.h:

  int  DLLEXPORT EN_getid(EN_Project ph, int object, int index, EN_ID *out_id);

where object can be EN_NODE, EN_LINK, etc. Then the body of EN_getid in epanet.c would look as follows:

int  DLLEXPORT EN_getid(EN_Project p, int object, int index, EN_ID *id)
{
    strcpy(*id, "");
    if (sizeof(*id) != EN_MAXID+1) return 252;  // May not be needed -- compiler will check this
    if (!p->Openflag) return 102;
    if (index < 1 ) return 203;
    switch (object) {
        case EN_NODE:
            if (index > p->network.Nnodes) return 203;
            strcpy(*id, p->network.Node[index].ID);
            break;
        case EN_LINK:  . . .

        default: return 251;
    }
    return 0;
}

A Toolkit client would use the new EN_getid function with code that looks something like:

EN_ID id;
int err, anIndex = . . . ;
. . .
err = EN_getid(p, EN_NODE, anIndex, &id);
printf("ID of Node[%d] is %s", anIndex, id);

A similar approach could be used to either modify the existing EN_getcomment function (breaking backward compatibility) or create a new function named EN_getcomment_s.

I've tested this safe method for retrieving ID names and it seems to work OK. What I'm not sure about is how to modify the various language bindings (epanet2.vb, epanet2.pas, etc.) so that they use the equivalent of EN_ID in their declaration of EN_getid.