working on an encryption problem

Question

0.00/5 (No votes)

See more:

It appears i'm not converting this from a char to a byte array this is working off of a resource and lpbuff contains a pe resource file any help would be great i'm an it sec guy looking to expand my knowledge to defend networks please help me with this

C#

char* TEXXOR2(char* lpBuf)
{
 int f = strlen(lpBuf);
 for (int i = 1;i<f;i++)
 {
     lpBuf[i] = int(char(lpBuf[i]) ^ 1230 % 15 * (36+1337 )*(22-1));
 }
 return lpBuf;
}

Edit: Got rid of the Windows and GUI tags as the question isn't about either

Posted 26-Apr-12 19:29pm

techvuln

Updated 26-Apr-12 21:14pm

Aescleal

v3

Add a Solution

2 solutions

Solution 2

In addition to the good advice of Aescleal I would like to give you the following hints:

It looks to me as if you are trying to implement a simple XOR encryption scheme, possibly something you have inherited from someone else. And now you are trying to decrypt several PE resources, but the encryption doesn't run correctly. Is that the situation you are in?

Here are a couple of possible reasons for things not working:

- The loop "for (int i..." should most likely start at 0, not at 1

- In the expression

lpBuf[i] ^ 1230 % 15 * (36+1337 )*(22-1)

I would have expected to be some component that depends on the loop variable "i". In the way it is written now, you are XORing and multiplying with the same arguments on every byte of the string. Could it be that you have mistaken the "i" in the original algorithm for a "1"?

- It looks like you are applying this to a binary resource, meaning not a text string. If that is so, then determining the length by strlen is no possible, because a 0-byte can occur anywhere in binary data. You would have to determine the length of the buffer that needs decryption in another way.

The lack of casting between char and BYTE is most likely not the problem.

Hope that any of these get you going.

Posted 26-Apr-12 22:38pm

nv3

Comments

Aescleal 27-Apr-12 5:27am

Good catch on the potential for multiple zeroes. Might be worth adding that he could pass in the size as a second parameter. Or just reinforce using a vector!

Cheers,

Ash

PS: Just realised that with operator precedence (I really must remember those rules but convince the world that brackets are good) the modulo is a fairly big number compared to the size of a character! I think that perhaps the data should be read 2 bytes at a time and the mod applied after XOR/exponentiation (it's about 29K, ~0x500).

nv3 27-Apr-12 5:48am

Thanks, Ash. And you are right with regard to operator precedence. I have a feeling that techvuln is trying to convert some old code and get things running again after a migration. So he probably wants to the algorithm do precisely what the old version did. Perhaps we could help better if we had the original code and then help translate that into the C function.

Aescleal 27-Apr-12 7:19am

I have the same feeling as well. I was probably a bit harsh in some of comments.

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Aescleal · Accepted Answer · 2012-04-26T21:10:00

At first glance I wasn't sure what you're up to here (I'm still not to be honest) as:-

- char(lpBuf[i]) is completely redundant - it's already a char
- you're XORing your character with a value more than than 255 (0xFF)
- the cast to int is also redundant as you immediately cast back to a char

Then I had a thought: As you say you're inexperienced. Are you using a crypto method (like RSA) which relies on exponentiation under modulus? If you are then you might think that ^ (the bitwise XOR operator) is actually exponentiation the way it is in a lot of other languages.

unsigned char initial_value = static_cast<unsigned char>( lpBuf[i] );
unsigned char value = initial_value;

for( int n = 2; n < 1230; ++n )
{
    val *= initial_val;
}

unsigned mod_denominator = 15 * (36 + 1337) * (22 - 1);
    lpBuf[i] = static_cast<char>( val % mod_denominator );

I might have still got this wrong... make sure you THOROUGHLY test any code you nick off me and understand why it works.

Even if that's what not you want then do yourself a favour or eight:

- Describe the algorithm you're trying to implement. If this is really something like RSA then it took me a while to work out what you're up to
- Better still use a standard implementation of a crypto algorithm. Don't do it yourself unless you're an expert and then why waste time doing it when other people have already done it and it's been reviewed?
- Break up complicated big expressions - there's no point in keeping it all on one line. I'd also move the loop into a different function as it's not clear what what's going on, I didn't bother here but probably should have done
- Know what the operators mean (you might already and I'm barking up the wrong tree) and understand operator precedence (the way I didn't when reading the code)
- Minimise casts. Compiler warnings are there for a reason! If you have to use casts use C++ style casts and NOT function style casts or C-style casts as they stick out like sore thumbs
- Use unsigned numbers for anyhing that acts on a stream of bytes or logical operations
- Don't use pointers, use std::vector
- Don't used magic numbers, EVER. Had you called 1230 exponent (if I'm right) or bit_mask (if I'm wrong) it would have helped no end. And presumably that collection of constants at the end is four numbers because they mean something in the context of the algorithm so name them with their meaning. Don't comment it either, call them what they are

I hope this lot helps,

Ash

Edit to improve the whole XORing a bit. Move along, nothing to see here apart from my dead ego :-) Fixed a few other bollocks up as well.