:: Re: [DNG] Systemd Shims
Top Pagina
Delete this message
Reply to this message
Auteur: Edward Bartolo
Datum:  
Aan: Roger Leigh
CC: dng
Onderwerp: Re: [DNG] Systemd Shims
I have corrected the way files are saved according to what I was
suggested. I also added preventive code to cause the program to exit
whenever essid are longer than 100 characters and passwords longer
than 200 characters. The program simply issues a error message telling
why it refused to execute the operation and terminates gracefully.

Now, I should think, the buffer overruns should not be possible, but I
am open to criticism. Buffer overruns are not something to be proud of
and correction when taken appropriately is a blessing.

Now, I will sleep as I am totally exhausted from coding all day long.

Edward.

On 19/08/2015, Roger Leigh <rleigh@???> wrote:
> On 19/08/2015 17:39, Rainer Weikusat wrote:
>
>> #define IFACE_TMPL \
>>     "auto lo\n" \
>>     "iface lo inet loopback\n\n" \
>>     "iface wlan0 inet dhcp\n" \
>>     "    wpa-ssid %s\n" \
>>     "    wpa-psk \"%s\"\n"

>>
>> #define IFACES_PATH "/tmp"
>>
>> static void saveFile(char* essid, char* pw) //argv[1], argv[2]
>> {
>>     char *path;
>>     FILE *fp;
>>     unsigned p_len, e_len;

>>
>>     p_len = strlen(IFACES_PATH);
>>     e_len = strlen(essid);
>>     path = alloca(p_len + e_len + 2);

>>     
>>     strcpy(path, IFACES_PATH);
>>     path[p_len] = '/';
>>     strcpy(path + p_len + 1, essid);

>>     
>>     fp = fopen(path, "ab+");
>>     fprintf(fp, IFACE_TMPL, essid, pw);
>>     fclose(fp);
>> }

>>
>> int main(int argc, char **argv)
>> {
>>     saveFile(argv[1], argv[2]);
>>     return 0;
>> }

>
> I'm not picking on this post in particular out of the rest of today's
> thread, but I did think this was a good example. While I don't want to
> act like a rabid C++ zealot, stuff like this really makes me shudder due
> to the fragility and unnecessary complexity for something which is
> really trivial.
>
> While the relative safety and security of C string handling can be
> debated, I do think the question needs asking: Why not use a language
> with proper safe string handling and avoid the issue entirely? It's
> only "safe" until it's refactored to break the existing assumptions and
> make it accidentally unsafe. The constants such as 2, 1 plus the
> strlen() calls are prime candidates for future bugs. It's not like this
> /needs/ to be done in C.
>
> void saveFile(const std::string& essid, const std::string& pw)
> {
>    std::string path(IFACES_PATH);
>    path += '/';
>    path += essid;

>
>    std::ofstream fp(path);
>    if (fp)
>    {
>      boost::format fmt(IFACE_TMPL);
>      fmt % essid % pw;
>      fp << fmt.str() << std::flush;
>    }
> }

>
> No leaks. No buffer overflows. Safe formatting. No file handle leaks.
> And it's readable--the intent is obvious since there's no extraneous
> buffer memory management. And it will compile down to something
> equivalent or even more efficient.
>
> If you use std::string you can still work directly with C functions
> using "const char *"--just use the .c_str() method and you get a
> suitable pointer.
>
> In my own code I use boost.filesystem, so rather than using "std::string
> path" you could then do
>
> path p = IFACES_PATH / essid;
>
> and have the path concatentation handled directly, and then use
> p.string() to get a string back. Even safer and more maintainable--work
> with path components directly rather than mangling strings.
>
> void saveFile(const std::string& essid, const std::string& pw)
> {
>    path p = IFACES_PATH / essid;
>    std::ofstream fp(p.string();
>    if (fp)
>    {
>      boost::format fmt(IFACE_TMPL);
>      fmt % essid % pw;
>      fp << fmt.str() << std::flush;
>    }
> }

>
> This is obviously easier and faster to write and maintain, so your
> energies are spent productively on the problem at hand, rather than
> faffing around with manual buffer management.
>
> And if efficiency isn't the prime consideration (and given the context,
> it isn't), then an interpreted language is likely an even better choice.
>
>
> Regards,
> Roger
> _______________________________________________
> Dng mailing list
> Dng@???
> https://mailinglists.dyne.org/cgi-bin/mailman/listinfo/dng
>

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <dirent.h>
#include <alloca.h>


//using namespace std;


#define opSave                0
#define opSaveConnect            1
#define opQueryConnect            2
#define opDeleteConnect            3
#define opConnectionConnect            4
#define opDisconnectActiveConnection    5
#define opScan                6
#define opLoadExisting            7



#define IFACE_TMPL \
        "auto lo\n" \
        "iface lo inet loopback\n\n" \
        "iface wlan0 inet dhcp\n" \
        "    wpa-ssid %s\n" \
        "    wpa-psk \"%s\"\n"


#define IFACES_PATH "/etc/network/wifi"


const
    char* path_to_interfaces_files = "/etc/network/wifi";

    
/*
1) Glib::spawn_sync instead of a pipe stream, provides a slot.
2) cmd trying to call an inexistent command still returns a valid pointer!
verify cmd exists before calling exec
*/

inline int file_exists(char* name) {
return (access(name, F_OK) != -1);
}


int exec(const char* cmd, char* out)
{
    const int buf_size = 128;

        
    FILE * pipe = popen(cmd, "r");
    char buffer[buf_size];
    while(!feof(pipe)) {
        if(fgets(buffer, buf_size, pipe) != NULL)
        {
            if (out != NULL)
                strcat(out, buffer);
                else strcpy(out, buffer);
        }
    }

        
    return pclose(pipe);
}


/*        Interfaces file sample
auto lo
iface lo inet loopback


# The primary network interface
# allow-hotplug eth0
iface eth0 inet dhcp


# WIFI Configuration
# auto wlan0
iface wlan0 inet dhcp
    wpa-ssid   ESSID
    wpa-psk   "password"


*/
    int saveFile(char* essid, char* pw) //argv[2], argv[3]
    {
        char *path;
        FILE *fp;
        unsigned p_len, e_len;


        p_len = strlen(IFACES_PATH);
        e_len = strlen(essid);
        path = alloca(p_len + e_len + 2);


        strcpy(path, IFACES_PATH);
        path[p_len] = '/';
        strcpy(path + p_len + 1, essid);


        fp = fopen(path, "ab+");
        fprintf(fp, IFACE_TMPL, essid, pw);
        fclose(fp);

    
        return 0;
    }

    
    
    int connectionConnect(char* essid) //argv[2]
    {
        char* s = 0;

        
        char ifilename[1024];
        strcpy(ifilename, path_to_interfaces_files);

        
        strcat(ifilename, "/");  
        strcat(ifilename, essid);

        
        char command[1024];
        strcpy(command, "/sbin/ifup wlan0 -i ");

        
        strcat(command, ifilename);
        printf(command);
        int q = exec(command, s);
        printf(s);
        return q;
    }

    
    int queryConnect(char* essid) //argv[2]
    {
        char s[50*1024];
        char command[1024];
        strcpy(command, "/bin/cat /etc/network/wifi/");
        strcat(command, essid);
        strcat(command, " | grep -A 1 wpa-ssid");
        int q = exec(command, s);
        printf(s);
        return q;
    }

    
    int deleteConnect(char* essid) //argv[2]
    {
        //char* s = 0;
        char command[1024];
        strcpy(command, "/bin/rm /etc/network/wifi/");
        strcat(command, essid);
        int q = exec(command, 0);
        //printf(s);
        return q;
    }

    
    int disconnectActiveConnection()
    {
        char* s = 0;

        
        char command[1024];
        strcpy(command, "/sbin/ifdown wlan0");
        int q = exec(command, s);
        printf(s);
        return q;
    } 

    
    int scan()
    {
        char s[50*1024];
        int u = exec("ifconfig wlan0 up", s);
        s[0] = '\0';    

        
        char command[1024];
        strcpy(command, "/sbin/iwlist wlan0 scanning | /bin/grep ESSID");
        int q = exec(command, s);
        printf(s);
        return q; 
    }

    
    int loadExisting()
    {
        DIR *dir;
        struct dirent *ent;

        
        if ((dir = opendir ("/etc/network/wifi")) != 0) 
        {
            while ((ent = readdir (dir)) != 0)
              if (!(strcmp(".", ent->d_name) == 0 || strcmp("..", ent->d_name) == 0))
                    printf ("%s\n", ent->d_name);

            
            closedir (dir);
            return 0;
        } 
        else 
        {
            perror ("");
            return EXIT_FAILURE;
        }
    }



// return value of -1 means operation not defined
// return value of -2 means incorrect number of parameter for operation selected
// return value of -3 means interfaces file name exceeds 100 characters
// return value of -4 means password exceeds 200 characters
int main(int argc, char *argv[])
{
    char *out = 0;
    int switch_item = -1, i;
    if (argc > 1)
    {
        // the first parameter must be a digit between 0 and 7
        // check length and converted value.
        if (strlen(argv[1]) == 1)
            switch_item = atoi(argv[1]);
        else {
            printf("Operation not defined\n");
            return -1;
        }    
        // atoi return zero if input is not a number
        // At this point we are sure argv[1] contains just one character
        if (switch_item == 0)
        {
            if (argv[1][0] != '0') {
                printf("Operation not defined\n");
                return -1;
            }
        }
    }

    
        
    // make sure parameters make it safe to proceed
    int valid = 0;
    if (argc == 2) {
            if ( 
                switch_item == opDisconnectActiveConnection    ||
                switch_item == opScan                                                ||
                switch_item == opLoadExisting
            ) valid = 1; // this means number of parameters is correct

            
            if (!valid) {
                printf("Incorrect number of parameter for operation selected.\n");
                return -2;
            }            
        }
    else if (argc == 3) {
            if ( 
                switch_item == opQueryConnect                ||
                switch_item == opDeleteConnect            ||
                switch_item == opConnectionConnect
            ) valid = 1; // this means number of parameters is correct

            
            if (!valid) {
                printf("Incorrect number of parameter for operation selected.\n");
                return -2; // again incorrect number of parameters
            }

            
            // here, we are checking the length of the ESSID
            // 52^100 permutions should be enough for a file name
            if (strlen(argv[2]) > 100) {
                printf("Interfaces file name exceeds 100 characters.\n");
                return -3; // interfaces file name too long
            }
        }
    else if (argc == 4) {
            if ( switch_item == opSave    || switch_item == opSaveConnect)
                valid = 1; // this means number of parameters is correct

            
            // again incorrect number of parameters
            if (!valid) {
                printf("Incorrect number of parameter for operation selected.\n");
                return -2; 
            }

            
            // 52^100 permutions should be enough for a file name
            if (strlen(argv[2]) > 100) {
                printf("Interfaces file name exceeds 100 characters.\n");
                return -3; // interfaces file name too long
            }

                
            // maximum password length of 200    
            if (strlen(argv[3]) > 200) {
                printf("Password exceeds 200 characters.\n");
                return -4;
            }    
        }


    
    switch (switch_item) {
        case opSave:
            i = saveFile(argv[2], argv[3]);

            
            //printf(out);
            return i;

            
        case opSaveConnect:
            i = saveFile(argv[2], argv[3]);
            if (i == 0) i = connectionConnect(argv[2]);
            return i;

            
        case opQueryConnect:
            i = queryConnect(argv[2]); 
            return i;

            
        case opDeleteConnect:
            i = deleteConnect(argv[2]);
            return i;

            
        case opConnectionConnect:
            i = connectionConnect(argv[2]);
            return i;

            
        case opDisconnectActiveConnection:
            i = disconnectActiveConnection();
            return i;

            
        case opScan:
            i = scan();
            return i;

            
        case opLoadExisting:
            i = loadExisting();
            return i;
    }

    
    return -1; // parameter not in range
}