I am currently using a byte encryption in C# which gets decrypted in my C++ client.
It is XOR, but I can't get it to work. Is there anything that Im doing wrong?
I already wrote the C++ XOR in C#, which look pretty much the same.
C#
Code:
public byte[] XOR(byte[] strng, byte[] key)
{
int string_len = strng.Length;
int key_length = key.Length;
int i, position;
for (i = 0; i < string_len; i++)
{
position = i % key_length;
strng[i] = Convert.ToByte(strng[i] ^ key[position]);
}
return strng;
}
C++
Code:
char *encrypt(char *string, char *key)
{
int string_len = strlen(string);
int key_length = strlen(key);
int i,position;
for(i = 0; i < string_len; i++)
{
position = i % key_length;
string[i] = (char)((int)string[i] ^ (int)key[position]);
}
return string;
}
In C# the bytes get encrypted like:
Code:
byte[] bytestocrypt = //here my source of the bytes
byte[] b = new byte[] { 4, 5, 3, 0 };
bytestocrypt = XOR(bytestocrypt, b);
I am currently using a byte encryption in C# which gets decrypted in my C++ client.
It is XOR, but I can't get it to work. Is there anything that Im doing wrong?
I already wrote the C++ XOR in C#, which look pretty much the same.
Pointers are not arrays. The code you wrote is not the same as the C# code. The C# code actually uses arrays. Pointers are just that in C++ -- pointers. They are not arrays (although you use pointers to point to the beginning of arrays). You can pass arrays to C# and they remain arrays, not so in C++, so the real solution is to use constructs that maintain their "arrayness" when passed around.
Second, post the main() program. Your attempt of using pointers is highly dependent on how you called that function. For example, this tells us nothing:
Code:
LPVOID pBuffer = //here source of decrypted bytes
Everything, and I mean everything depends on how you initialized that pBuffer.
Third, you posted another question concerning this, and my response to you is the same -- ditch the pointers and use classes. Your C++ code not only is not the same, it is dangerous as heck. I can easily break it like this:
Code:
char *encrypt(char *string, char *key);
//..
encrypt("abc123", "123456"); // writing to a string literal as the first argument -- possible crash
encrypt(NULL, NULL); // calling strlen() on a NULL pointer -- undefined behaviour
Both of those lines compile. Now what happens when you run it? The first one has a string-literal as the first argument. The second has a NULL as the first argument. Providing those values as arguments makes your encrypt function go haywire.
I could go on and on about how that function is crash-prone. You're using C++, not 'C' -- there is absolutely no excuse to make crash-prone functions like this. To make it more bullet-proof, use what I mentioned in the other thread, and that is container classes.
Code:
#include <vector>
typedef std::vector<char> ByteArray;
//...
ByteArray& XOR(ByteArray& strng, const ByteArray& key)
{
size_t string_len = strng.size();
size_t key_length = key.size();
int position;
for (size_t i = 0; i < string_len; i++)
{
position = i % key_length;
strng[i] = (ByteArray::value_type)(strng[i] ^ key[position]);
}
return strng;
}
This code is much more safer to use. Also, here is a radically different way in C++ to do the same code.
The utility of doing it this way may not be apparant, but basically, I created a function where you can plug-in the transformation you want to use. If you come up with another way to transform the source string, just call:
Code:
Transform<AnotherWay>(barr, key);
Where AnotherWay is a class that handles the key and transforms the source string in whatever way it sees fit.
Regards,
Paul McKenzie
Last edited by Paul McKenzie; May 31st, 2012 at 03:32 PM.
That is indeed right, I could not get the other version working. So this time, instead of translating C# to C++, I chose a snippet which was posted online.
I translated this into C# because that is the language that I normally use to code, and understand.
Your solution seems to be right, but I would like to use an algorithm instead of these libraries to XOR.
This XOR snippet was posted online and seemed to work for the people.
And thanks for pointing out the problems.
---
Btw I used your above snippet and there still seems to be an error on the first line, passing the first argument.
And the source of the Buffer is just a bytearray from RC DATA embedded resources. (FindResource(0, "0", "RT_RCDATA");
Your solution seems to be right, but I would like to use an algorithm instead of these libraries to XOR.
Where did I not use the algorithm you posted? It's in both the examples I posted -- you just don't recognize it.
Btw I used your above snippet and there still seems to be an error on the first line, passing the first argument.
The full example I posted works correctly. I don't know what you're doing or what data you're using to have such an error.
And the source of the Buffer is just a bytearray from RC DATA embedded resources. (FindResource(0, "0", "RT_RCDATA");
OK, so what's the problem? You can take any byte array and place it in the vector.
That's why I mentioned to you in the other thread to learn the interface of the vector class. If you did that, you wouldn't see that there is any problem.
Regards,
Paul McKenzie
Last edited by Paul McKenzie; May 31st, 2012 at 08:41 PM.
|45|error: invalid initialization of reference of type 'ByteArray& {aka std::vector<char>&}' from expression of type 'char*'|
|13|error: in passing argument 1 of 'ByteArray& XOR(ByteArray&, const ByteArray&)'|
Sorry I did recognize it of course, but I just wanted to say that the second example that you coded was not an option for me.
Well, it is the same as the first example, except it's wrtten in a much more flexible way.
Well if it is working for you, then I will be making a mistake?
This is the full code that I am talking about.
Even if you didn't use ByteArray, there are problems with your code.
First, All of those Windows API calls are off-topic in this forum. Assume I know nothing about them or what they do.
Code:
LPVOID pBuffer = NULL;
//...
pBuffer = lpData;
So is pBuffer NULL terminated array of char? If not, then your original XOR function using char* would not have worked properly, since calling strlen() on data you aren't sure is NULL terminated leads to strlen() going off the deep-end trying to find the NULL.
If it is NULL terminated, then you need to create ByteArray's from this data. You can't just send it to the function, as raw char data is not a ByteArray. Remember that ByteArray is a vector -- a full-blown class that wraps an array, and isn't just an array. You have to get the raw data into the ByteArray object somehow. That's why I mentioned to you to study the interface of vector, so that you know how to convert from a raw type to a vector:
Then you use b1 and b2 from there, as pBuffer is no longer needed at that point.
There is a constructor for vector that takes two arguments, where both arguments are iterators to starting and beginning of the sequence of values. In the example, you want to create a vector that consists of characters from pBuffer and from b.
The b1 vector is created by supplying the start of pBuffer and the end of pBuffer. The same with b2 -- the start of b and the end of b are provided. Then presto, you have the vector of characters.
Again, I'm assuming you know that pBuffer is NULL terminated. If it isn't, then you need to know the number of bytes so as to set up the b1 vector correctly (you can't use strlen()).
Nice design, but your XORFunction object carries a variable state, which makes it implicitly dangerous, especially when mixed with the STL (transform does not necessarily guarantee the range will be transformed in-order if you provide it with a random access iterator, for example).
IMO, the Encoder should take an entire input range directly anyways, as some encoders will need the entire input to function correctly anyways.
Did you purposefully omit to pass a Transformer object? I suppose it was to prevent the re-use of the XORFunction (which would be problematic, given the whole state thing)?
Function-like parameters parameters are usually also passed as an argument, in case they can be parametrized (as far as I have seen anyways). Plus, it avoids having to specify the parameterization of the function when calling it.
Is your question related to IO?
Read this C++ FAQ LITE article at parashift by Marshall Cline. In particular points 1-6.
It will explain how to correctly deal with IO, how to validate input, and why you shouldn't count on "while(!in.eof())". And it always makes for excellent reading.
Nice design, but your XORFunction object carries a variable state, which makes it implicitly dangerous, especially when mixed with the STL (transform does not necessarily guarantee the range will be transformed in-order if you provide it with a random access iterator, for example).
True. I was assuming the iterator was random access and did an in-order traversal (from begin() to end() in that order).
Did you purposefully omit to pass a Transformer object? I suppose it was to prevent the re-use of the XORFunction (which would be problematic, given the whole state thing)?
Not on purpose, no. I could have rewritten it to take a Transformer as a parameter also. I was thinking more of the lines of a policy-based design.
All you're saying is true however.
Edit:
Regards,
Paul McKenzie
Last edited by Paul McKenzie; June 1st, 2012 at 12:22 PM.
Uhm yes I see the difference, but I have no idea why it would be dangerous.
Are you saying that the difference between
char p[] and char *p is that char p[] is really a char array while the first one is a string-literal and not writable?
Uhm yes I see the difference, but I have no idea why it would be dangerous.
Are you saying that the difference between
char p[] and char *p is that char p[] is really a char array while the first one is a string-literal and not writable?
Yes. Why not try the small program and see what happens? Depending on the compiler you're using, the code that uses the string-literal will immediately crash on the assignment to p[0].
Regards,
Paul McKenzie
Last edited by Paul McKenzie; June 4th, 2012 at 09:14 PM.
Oh finallly got the difference now then, thanks. Indeed also on stackoverflow a good example. The first one is in the read-only memory without copying the string to the newly allocated memory.
So next step would be
Code:
char *XOR(char str[], char cipher[])
{
for (size_t i = 0; i < strlen(str); i++)
{
str[i] ^= cipher[i % strlen(cipher)];
}
return str;
}
Not sure though if the call stays the same, as that is still unwritable and does not need to be writable. so just kept the (char *) there.
char *XOR(char str[], char cipher[])
{
for (size_t i = 0; i < strlen(str); i++)
{
str[i] ^= cipher[i % strlen(cipher)];
}
return str;
}
No. Still wrong.
I also mentioned this to you on the other thread. This
Code:
char *XOR(char str[], char cipher[])
is exactly the same as this:
Code:
char *XOR(char *str, char *cipher)
So you haven't changed a thing.
Ill Google now to see what you mean by the NULL first argument.
You are calling strlen(NULL) in your function if NULL is the first argument. There is no guarantee how strlen() behaves if a NULL is passed, as strlen() doesn't check for NULL pointers. All strlen() does is start at the address you give it, and search for a '\0' character.
A NULL address is not the same thing as a '\0' character.
In your function, you're assigning a value to str, which is the first parameter. You are passing an unassignable string-literal as the first parameter. Stop passing string literals and instead pass the address of the first element of a true char array.
C++ is not C#, where you get all kinds of warning bells and flags going off when you make a mistake. You are basically on your own, and C++ allows you to shoot yourself in the foot if you want to.
I don't think you're aware of these things -- in C++, just because something compiles doesn't mean it is a well-formed program, and the only issue is that the logic may be off. Nothing could be further from the truth -- you can create ill-formed programs that compile that break all sorts of rules (writing to string literals, accessing array elements out of bounds, etc.), thus the behaviour of such programs is -- you don't know.
You are calling strlen(NULL) in your function if NULL is the first argument. There is no guarantee how strlen() behaves if a NULL is passed, as strlen() doesn't check for NULL pointers. All strlen() does is start at the address you give it, and search for a '\0' character.
A NULL address is not the same thing as a '\0' character.
Regards,
Paul McKenzie
But my first argument will not be NULL, so I dont see the problem in this.
But my first argument will not be NULL, so I dont see the problem in this.
And if I use your function and pass a NULL, it breaks.
What if in another (larger) program, the string you're passing it is generated or computed at runtime (in other words, you have no way to know in advance what the string will be), and that generation produces a NULL. You then blindly pass it to your XOR function. What then?
The point being that a function should be written so that simple things like this won't break it. When you write a function, the underlying reason why you write it is that it can be reused in other programs and other situations, not just the situation you find yourself in now.
Regards,
Paul McKenzie
Last edited by Paul McKenzie; June 4th, 2012 at 10:00 PM.
Reply With Quote
Originally Posted by qwerz
Bookmarks