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 <rle...@codelibre.net> 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@lists.dyne.org
> 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
}_______________________________________________
Dng mailing list
Dng@lists.dyne.org
https://mailinglists.dyne.org/cgi-bin/mailman/listinfo/dng
