mersenneforum.org

mersenneforum.org (https://www.mersenneforum.org/index.php)
-   Programming (https://www.mersenneforum.org/forumdisplay.php?f=29)
-   -   Official "String copy Statement Considered Harmful" thread (https://www.mersenneforum.org/showthread.php?t=16859)

Dubslow 2012-05-27 15:58

Official "String copy Statement Considered Harmful" thread
 
1 Attachment(s)
[QUOTE=flashjh;300387]Probably should have them already, do you have a link to mfaktc source files?[/quote][url]http://mersenneforum.org/mfaktc/[/url] (The unqualified 0.18 would be the source.)
[QUOTE=flashjh;300387]
I'm pretty sure I need _strnicmp instead of strncasecomp and strcpy_s instead of strcpy, [/quote]
I don't see anywhere in the mfatkc source about strcpy_s, but I'll keep looking, and you certainly know better than me about MSVC :smile:
Edit: [url]http://msdn.microsoft.com/en-us/library/8ef0s5kh(v=vs.80).aspx[/url]
[quote=MS]For example, the strcpy function has no way of telling if the string that it is copying is too big for its destination buffer. However, its secure counterpart, strcpy_s, takes the size of the buffer as a parameter, so it can determine if a buffer overrun will occur. If you use strcpy_s to copy eleven characters into a ten-character buffer, that is an error on your part; strcpy_s cannot correct your mistake, but it can detect your error and inform you by invoking the invalid parameter handler.[/quote]
strcpy_s has a different definition, requiring a third size argument; it's just meant to be a "catch stupid programmer error" thing. Since the code does a check for a long line anyways, the extra functionality is unnecessary.
[code]bill@Gravemind:~/CUDALucas/test∰∂ cat parse.c
#include <stdio.h>
#include <string.h>
#include <limits.h>
#include <ctype.h>
#include <errno.h>
#include <stdlib.h>

[B]#ifdef _MSC_VER
#define strncasecmp _strnicmp
#define _CRT_SECURE_NO_WARNINGS
#endif[/B]

int isprime(unsigned int n)
/*
returns
0 if n is composite
1 if n is prime
*/
{
unsigned int i;

if(n<=1) return 0;
if(n>2 && n%2==0)return 0;

i=3;
while(i*i <= n && i < 0x10000)
{
if(n%i==0)return 0;
i+=2;
}
return 1;
}[/code]
[QUOTE=flashjh;300387]however, I don't know what to do with the 'conditional expression is constant'.[/QUOTE]Oh, that's just a while(1), that warning can be ignored. (Perhaps deleting the 1 and leaving the conditional blank will work?) Edit: Actually, in further looking, I actually have no clue why that loop is there at all. If the func finds a too-long line, then it reads in the characters until a control character then breaks? Wtf?


PS I forgot to mention above, but the help message was moved from no-args to the -h arg.
PPS Here's a slightly larger .ini file:
[code]# Polite is the same as the -polite option. If it's 1, each iteration is
# polite. If it's (for example) 12, then every 12th iteration is polite. Thus
# the higher the number, the less polite the program is. Set to 0 to turn off
# completely. Polite!=0 will incur a slight performance drop, but the screen
# should be more responsive. Trade responsiveness for performance. [U](Note:
# polite=0 is known to cause CUDALucas to use some extra CPU time; Polite=64 or
# higher is a good compromise.)[/U]
Polite=1[/code]
flash, could you add this to your copy before you bundle the executable?

The attached archive contains the modified parse.c and CUDALucas.ini.

LaurV 2012-05-27 16:45

[QUOTE=Dubslow;300390]
strcpy_s has a different definition, requiring a third size argument; it's just meant to be a "catch stupid programmer error" thing. [/QUOTE]

[offtopic]That is totally false. It is meant to catch malicious guys feeding you with a malicious string [U][B]at run time[/B][/U]. Have a look at [URL="http://www.google.co.th/search?q=buffer+overflow+example&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:official&client=firefox-a"]buffer overflow thingies[/URL], and plenty of viruses/trojans who exploit it. Classical strcpy will copy a string till a \0 is found. If there is not one found, all your memory could be overwritten. As both the string (data) and the code are in the memory, if your program does not take special precautions, then I can make a malicious string that will be copied over (overwrite) part of your program. Please make a habit to use "safe" string functions every time you can (that should be 99.99%of the cases). If I know where your string is in memory (easy to find out, search the common buffers for ascii characters) then I can replace the \0 and you program goes in the woods.[/offtopic]

Dubslow 2012-05-27 16:50

[QUOTE=LaurV;300394][offtopic]That is totally false. It is meant to catch malicious guys feeding you with a malicious string [U][B]at run time[/B][/U]. Have a look at [URL="http://www.google.co.th/search?q=buffer+overflow+example&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:official&client=firefox-a"]buffer overflow thingies[/URL], and plenty of viruses/trojans who exploit it. Classical strcpy will copy a string till a \0 is found. If there is not one found, all your memory could be overwritten. As both the string (data) and the code are in the memory, if your program does not take special precautions, then I can make a malicious string that will be copied over (overwrite) part of your program. Please make a habit to use "safe" string functions every time you can (that should be 99.99%of the cases). If I know where your string is in memory (easy to find out, search the common buffers for ascii characters) then I can replace the \0 and you program goes in the woods.[/offtopic][/QUOTE]
Ah, thanks for that clarification. Your choice, flash. Strings are 100 chars (plus null terminator) in parse.c.

Dubslow 2012-05-27 20:58

1 Attachment(s)
Try this on for size:
[code]bill@Gravemind:~/CUDALucas/test∰∂ cat parse.c

#include <stdio.h>
#include <string.h>
#include <limits.h>
#include <ctype.h>
#include <errno.h>
#include <stdlib.h>

#ifdef _MSC_VER
#define strncasecmp _strnicmp
#endif

void strcopy(char* dest, char* src, size_t n)
{
#ifdef _MSC_VER
strcpy_s(dest, n, src);
#else
strncpy(dest, src, n);
#endif
}

int isprime(unsigned int n) //...etc[/code]
Turns out the Linux "safe" equivalent is strncpy, much like strncasecmp. It's too bad Microsoft can't just use the same damn function. (Note the different ordering of arguments, as well as the name.)

Where it gave warnings, replace those "strcpy" calls with "strcopy", and add a third argument "MAX_LINE_LENGTH+1". Alternately, just DL the attached modified version :smile:

flashjh 2012-05-28 05:28

I'll work compiling tomorrow. I have to get some :sleep:

kjaget 2012-05-30 14:33

[QUOTE=Dubslow;300430]Try this on for size:
[code]bill@Gravemind:~/CUDALucas/test∰∂ cat parse.c

#include <stdio.h>
#include <string.h>
#include <limits.h>
#include <ctype.h>
#include <errno.h>
#include <stdlib.h>

#ifdef _MSC_VER
#define strncasecmp _strnicmp
#endif

void strcopy(char* dest, char* src, size_t n)
{
#ifdef _MSC_VER
strcpy_s(dest, n, src);
#else
strncpy(dest, src, n);
#endif
}

int isprime(unsigned int n) //...etc[/code]
Turns out the Linux "safe" equivalent is strncpy, much like strncasecmp. It's too bad Microsoft can't just use the same damn function. (Note the different ordering of arguments, as well as the name.)

Where it gave warnings, replace those "strcpy" calls with "strcopy", and add a third argument "MAX_LINE_LENGTH+1". Alternately, just DL the attached modified version :smile:[/QUOTE]

strncpy() isn't a safe version of strcpy(). It doesn't report errors and will not nul-terminate a string if it hits the size limit. That means it itself won't cause buffer overruns, but any subsequent use of the results of likely will if your source string was too long.

LaurV 2012-05-30 14:54

[QUOTE=kjaget;300705]strncpy() isn't a safe version of strcpy().[/QUOTE]
True. strncpy() is not a "version" that strenghten the "safety" of strcpy. It is a DIFERENT function. And it is a SAFE function.

Dubslow 2012-05-30 19:49

[QUOTE=kjaget;300705]strncpy() isn't a safe version of strcpy(). It doesn't report errors and will not nul-terminate a string if it hits the size limit. That means it itself won't cause buffer overruns, but any subsequent use of the results of likely will if your source string was too long.[/QUOTE]

I see, thanks for pointing that out as well. (strcpy_s() does guarantee a null-terminated string.) However, strncpy is certainly safer than strcpy, and I don't know of any other alternatives on Linux. (And, as I said, the code in question does check for a long line anyways, so even if it's not terminated it would be caught.)

chalsall 2012-05-30 20:00

[QUOTE=kjaget;300705]strncpy() isn't a safe version of strcpy(). It doesn't report errors and will not nul-terminate a string if it hits the size limit. That means it itself won't cause buffer overruns, but any subsequent use of the results of likely will if your source string was too long.[/QUOTE]

[code]strncpy(dest, src, length);
dest[length-1]=0;[/code]

...is pretty safe....

Dubslow 2012-05-30 20:05

[QUOTE=chalsall;300731][code]strncpy(dest, src, length);
dest[length-1]=0;[/code]

...is pretty safe....[/QUOTE]

But you don't know for sure without further code if some part of src got clobbered. [URL="http://msdn.microsoft.com/en-us/library/td1esda9(v=vs.80).aspx"]strcpy_s[/URL] throws an error if dest is too small for src.
[quote=MSFT]If strDestination or strSource is a null pointer, or [U]if the destination string is too small[/U], the invalid parameter handler is invoked as described in Parameter Validation. If execution is allowed to continue, [U]these functions return EINVAL and set errno to EINVAL.[/U]

Upon successful execution, the destination string will always be null terminated.[/quote]

chalsall 2012-05-30 20:22

[QUOTE=Dubslow;300733]But you don't know for sure without further code if some part of src got clobbered.[/QUOTE]

src wouldn't get clobbered; dest simply wouldn't have the full string.

[code]int strcpy_s(
char *dest,
int n,
const char *src
)
{
if (!dest || !src) {
return 1; // Stupid human...
}

if (strlen(src) > n-1) {
return 2; // dest too small; Stupid human...
}

strncpy(dest, src, n);

return 0;
}[/code]


All times are UTC. The time now is 15:16.

Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2021, Jelsoft Enterprises Ltd.