Re: [PATCH 1/2] kernel/tests: Implement safe local strcat function.
"Saulius Krasuckas" <saulius.krasuckas(a)ieee.org> wrote:
+static void safe_strcatA(char *dst, DWORD dst_len, const char *src) +{ + UINT length; + char *buff; + + length = lstrlenA(dst); + length += lstrlenA(src); + buff = HeapAlloc(GetProcessHeap(), 0, length + 1); + + lstrcpyA(buff, dst); + lstrcatA(buff, src); + lstrcpynA(dst, buff, dst_len); + + HeapFree(GetProcessHeap(), 0, buff); +}
This kind of "safe" wrappers doesn't makes the code be safer in reality, needlessly clobbering the code instead. -- Dmitry.
* On Tue, 4 Jul 2006, Dmitry Timoshkov wrote:
* "Saulius Krasuckas" <saulius.krasuckas(a)ieee.org> wrote:
+static void safe_strcatA(char *dst, DWORD dst_len, const char *src) +{ + UINT length; + char *buff; + + length = lstrlenA(dst); + length += lstrlenA(src); + buff = HeapAlloc(GetProcessHeap(), 0, length + 1); + + lstrcpyA(buff, dst); + lstrcatA(buff, src); + lstrcpynA(dst, buff, dst_len); + + HeapFree(GetProcessHeap(), 0, buff); +}
This kind of "safe" wrappers doesn't makes the code be safer in reality, needlessly clobbering the code instead.
By "safe" I meant it doesn't overflow buffer. Does it do? But well, would your reactions was the the same if I'd called this local_strcatA(), uhm?
"Saulius Krasuckas" <saulius2(a)ar.fi.lt> wrote:
By "safe" I meant it doesn't overflow buffer. Does it do?
There are other ways to achieve that. Once Alexandre already commented out an almost the same approach someone tried to port from ReactOS to Wine.
But well, would your reactions was the the same if I'd called this local_strcatA(), uhm?
My reaction has nothing to do with the function name. -- Dmitry.
* On Tue, 4 Jul 2006, Dmitry Timoshkov wrote:
* "Saulius Krasuckas" <saulius2(a)ar.fi.lt> wrote:
By "safe" I meant it doesn't overflow buffer. Does it do?
There are other ways to achieve that. Once Alexandre already commented out an almost the same approach someone tried to port from ReactOS to Wine.
Hm, I don't remember this. Can you give me an example of a nice way, please? It was one "if" that I removed in my patch. Maybe you mean this way? If so, I don't understand how embracing every testable function call by an "if" statement looks better than local strcat-wrapper.
"Saulius Krasuckas" <saulius2(a)ar.fi.lt> wrote:
There are other ways to achieve that. Once Alexandre already commented out an almost the same approach someone tried to port from ReactOS to Wine.
Hm, I don't remember this. Can you give me an example of a nice way, please? It was one "if" that I removed in my patch. Maybe you mean this way?
Yes, that's a perfect way of checking for a possible buffer overflow, there is no need to complicate things.
If so, I don't understand how embracing every testable function call by an "if" statement looks better than local strcat-wrapper.
strcat wrapper creates an illusion that it solves some problem when it actually doesn't. -- Dmitry.
* On Tue, 4 Jul 2006, Dmitry Timoshkov wrote:
* "Saulius Krasuckas" wrote:
It was one "if" that I removed in my patch. Maybe you mean this way?
Yes, that's a perfect way of checking for a possible buffer overflow, there is no need to complicate things.
It may be perfect from some POVs, but it creates some conditional branching which I was thinking we're are trying to avoid in WRT suite, for example branching used in checking for version-dependent stuff.
strcat wrapper creates an illusion that it solves some problem when it actually doesn't.
So you agree there is a problem which isn't solved in current test code?
"Saulius Krasuckas" <saulius2(a)ar.fi.lt> wrote:
strcat wrapper creates an illusion that it solves some problem when it actually doesn't.
So you agree there is a problem which isn't solved in current test code?
I have no idea how it's possible to make such a conclusion from what I've said. -- Dmitry.
* On Wed, 5 Jul 2006, Dmitry Timoshkov wrote:
* "Saulius Krasuckas" wrote:
strcat wrapper creates an illusion that it solves some problem when it actually doesn't.
So you agree there is a problem which isn't solved in current test code?
I have no idea how it's possible to make such a conclusion from what I've said.
As this already doesn't relate to development, I am switching to private mode ;)
participants (2)
-
Dmitry Timoshkov -
Saulius Krasuckas