• We’re currently investigating an issue related to the forum theme and styling that is impacting page layout and visual formatting. The problem has been identified, and we are actively working on a resolution. There is no impact to user data or functionality, this is strictly a front-end display issue. We’ll post an update once the fix has been deployed. Thanks for your patience while we get this sorted.

Why do people even bother to try writing encryption algorithms?

M

Mark R

Diamond Member
I was recently reviewing an electronic medical records and telemedicine package that I was planning to buy for a small remote clinic. The vendor kindly supplied me with a demo copy of the package so that I could evaluate it.

All looked good. I enquired about security - AES 256 throughout, the CEO told me.

Anyway curiosity got the better of me and I fired up my JustDecompile and decided to take a look.

Code: 
public static string encrypt(string encrypt)
{
    string str;
    string empty = string.Empty;
    try
    {
        string str1 = encrypt.TrimEnd(new char[0]);
        char[] charArray = str1.ToCharArray();
        int num = 0;
        while (true)
        {
            bool length = num < (int)charArray.Length;
            if (!length)
            {
                break;
            }
            int num1 = Convert.ToInt32(charArray[num]) + 10;
            charArray[num] = Convert.ToChar(num1);
            empty = string.Concat(empty, charArray[num].ToString());
            num++;
        }
        str = empty;
    }
    catch (Exception exception)
    {
        str = null;
    }
    return str;
}

Bwahahahah.
 
Oh man, you have to be kidding me. Are you sure that's the routine they're calling? Maybe that's old code.
 
I'm not familiar with the language but if I'm reading it correctly it is a ROT 10 routine?

Edit: Does that {str = null} at the end just empty the string no matter what?
 
Last edited:
IronWing said:
I'm not familiar with the language but if I'm reading it correctly it is a ROT 10 routine?

Edit: Does that {str = null} at the end just empty the string no matter what?
Click to expand...

str = null is only if the function has an error. The try/catch statement encapsulates the logic.
 
Wow. Took me a bit there to recognize that it wasn't as completely pointless as it first seemed, since str empty was not in fact empty.

And then on third glance, it is as completely pointless as it first seemed.

Sirius Cybernetics style code - the superficial flaws hide the fundamental ones.
 
I wouldn't even call that encryption, it's closer to basic encoding than an actual encryption algorithm, offers almost no security at all.
 
This could be just a part of encryption. If that's something like an additional encryption before AES starts, it falls in place...
 
Only if attacker knows of it. Anyway, pre-encryption and pre-compression are quite a different actions.
 
It took me awhile to realize that str=empty did not assign str="".

Once I got past that, I realized this is a really bad encryption method. One might even call its result ROT-TEN encryption. 😉
 
It does amaze me because .net has hundreds of pre-written, pre-tested encryption algos already that you don't need to write any code for and probably still perform better than this one.

I would double-check to make sure that is the function they are using and that you can decrypt some data using the reverse algo. Its possible that is old code as markbnj mentions.

The big problem is that if you mention this to the CEO, he is likely to sue you under DMCA. You may need to send an anonymous letter instead.
 
They definitely use this function for some stuff (mainly storing some stuff on the client computer), but a more thorough inspection shows that they do indeed use AES256 for transmitting some data, with others bits of data encrypted with 3DES (mainly the software license/serial).

I didn't get a copy of the server software so don't know how it's stored on the server.

Personally, rather than encrypting bits of transmitted data manually - I'd have just used a TLS connection. Considering that the whole thing was written in .NET and used the WCF transparent client/server API, TLS support was literally a matter of changing the http:// to https:// in the service definition and attaching a self-signed certificate.
 
Last edited:
Mark R said:
Personally, rather than encrypting bits of transmitted data manually - I'd have just used a TLS connection. Considering that the whole thing was written in .NET and used the WCF transparent client/server API, TLS support was literally a matter of changing the http:// to https:// in the service definition and attaching a self-signed certificate.
Click to expand...

True, and that will protect the data in transit, but this algorithm may have been intended to encrypt data at the field level for storage purposes.
 
People rewrite existing code all the time. One of the most useful things I've done was to quickly look at the documention for a bunch of common libraries (Google, IBM, Apache, etc). It's saved me a ton of time. Just today, I needed to generate random strings and I used RandomStringUtils from Apache commons-lang.
 
Last edited:
Markbnj said:
True, and that will protect the data in transit, but this algorithm may have been intended to encrypt data at the field level for storage purposes.
Click to expand...

If they wanted to encrypt at the storage level they should have used database encryption, not this messy function. If the database didn't support encryption they could have still used encryption functions to encrypt the strings before putting them in the database.
 
KB said:
If they wanted to encrypt at the storage level they should have used database encryption, not this messy function.
Click to expand...

Well yeah, no kidding 🙂. I was just pointing out that TLS doesn't cover security across the whole information domain. My implicit assumption was that the method above could not possibly be intended to secure information across the transport layer, because nobody could be that stupid.
 
You must log in or register to reply here.

TRENDING THREADS

Back
Top