• Drusenija (unregistered) in reply to James
    James:
    Actually, the MD5 version above *could* be a less-than-terrible idea, especially if the password is taken from user input at runtime (and not stored anywhere on disk in plaintext).

    Sure, you'd be better off pulling the hash from someplace external (with write access controlled by the OS). But the OP didn't say whether this was for an "enterprise" system or just something used in-house. Plenty of times, I've written an in-house app where myself and a small team are using e.g. a tracking database where I, as the developer, can do some "power user" things with which I don't want the worker-bee users to shoot themselves in the foot. In such cases -- where it's not critical security, but rather an elaborate "are you sure" prompt -- I might find myself using a trick like this.

    I guess calling it "a trick like this" kind of admits that it's a WTF, though.

    I actually agree with you here; I've done this for some small internal projects that do things like a complete database initialization, etc. As James says, it's an elaborate are you sure style prompt.

    However, this is not security ^_~

  • utunga (unregistered) in reply to rob
    rob:
    I with TheRubyWarlock, this is a fake.

    Nah.. I could totally buy this happening. Its undeniably stupid, but I'd certainly believe that there is someone that realises that they don't want code out there messing with his 'isModified' function but at the same time doesn't know about the value of 'internal'. In my experience lots of perfectly good C# programmers don't grok the meaning of internal type modifiers initially. (yes, and myself).

    Also, even if you know about internal, sometimes its not quite enough.

    I've briefly contemplated this type of insane solution (soon to realise that it is in fact a total wtf) when you have a situation where you want one and only class - in fact specifically an object factory in this case - to be able to call a SetClean() method just like this example.

    In that situation the internal keyword doesn't quite cut it because you want only that factory class to be able to call the SetClean() function, not just anything in the assembly. C++ 'friend' would do the job here but C# doesn't have "friend". Still at a certain point you have to stop being so dang paranoid.

    But yeah, I'd definitely believe this is not fake.

  • EdMalloyHater (unregistered) in reply to TheRubyWarlock

    This code HAD to be concieved by Ed Malloy.

  • Hehehe (unregistered)

    /// Overrides the IsUpdated value. Used to set the value to false /// after properties have been populated by the constructor.

    Umm, isn't the real WTF that the guy didn't just set updated to false at the end of the constructor?

  • iMalc (unregistered)

    I've seen this kind of thing before in functions exposed in DLLs, only it was a magic number, not a string. This makes less sense though.

  • ch__ (unregistered)

    Wow. Killer idea, actually.

  • (cs) in reply to Joe Grossberg
    Joe Grossberg:
    Jeebus ... can we just retire the "hard-coded, plain-text password" WTF already?

    This site is getting repetitive, and it was only funny the first 100 times.

    Jeebus... Can we just retire the whiners who bitch and moan about the posts? It's getting really repetitive, and it wasn't funny even the first time, much less the 100th.

    Seriously... If you don't like the damned articles, don't read them. Wasting your time and ours whining just demonstrates what a moron you are, and accomplishes nothing else.

  • Look at me! I'm on the internets! (unregistered) in reply to KenW
    KenW:
    Joe Grossberg:
    Jeebus ... can we just retire the "hard-coded, plain-text password" WTF already?

    This site is getting repetitive, and it was only funny the first 100 times.

    Jeebus... Can we just retire the whiners who bitch and moan about the posts? It's getting really repetitive, and it wasn't funny even the first time, much less the 100th.

    Seriously... If you don't like the damned articles, don't read them. Wasting your time and ours whining just demonstrates what a moron you are, and accomplishes nothing else.

    Jeebus. Can we just retire the the moaners who bitch and whine about the whinerer who bitch and moan about the posts? It's getting really recursive.

  • M L (unregistered)

    I have to admit, I'm thoroughly enjoying the long, drawn out conversation on the use of == when Alex made it clear already that Alex introduced that artifact himself and wasn't in the original code!

    Note from Alex: Sorry, this was a transcription error. I fixed it. The original code was identical with the exception of using string.compare(). I changed it to != (well actually == by mistake) b/c it made the WTF a lot easier to immediatley see.
  • IWroteThis (unregistered)

    I wrote the following a short while ago:

    #
# If someone comes along and screws up the following
# command, you will remove EVERY file on the system
# that you are allowed to.  If you are not %100 sure
# (or %110), DO NOT CHANGE THE FOLLOWING COMMAND.
#
#

if [ "$IS_JOINED_DIR/" = "/" ]
        then
                echo ERROR! You MUST execute this from menu control.
                echo "(bozo)"
                exit 1
        fi
  • Brian Reiter (unregistered)

    This is .NET, the author clearly should have used code access security attribributes instead of a password.

    [PrincipalPermission(SecurityAction.Demand, Role = "UpdateStatusFlippers")] OverrideIsUpdated( bool status ) { _IsUpdated = status; }

  • (cs) in reply to M L
    M L:
    I have to admit, I'm thoroughly enjoying the long, drawn out conversation on the use of == when Alex made it clear already that Alex introduced that artifact himself and wasn't in the original code!
    Note from Alex: Sorry, this was a transcription error. I fixed it. The original code was identical with the exception of using string.compare(). I changed it to != (well actually == by mistake) b/c it made the WTF a lot easier to immediatley see.

    I also think he shouldn't have changed it in the first place. This "change to make it easier to see" spawned a whole discussion on it's own even if you ignore the part about the ==/!= mixup.

  • Miles (unregistered) in reply to Neerav

    The fact that they're using a "String" object, and that, according to alex, it originally was a comparator method, not '==', makes me think it's not C#... or if it is, apparently the coder was also unaware that C# can handle 'string' (lowercase) as if it were a primitive type.

  • OhMyGod! (unregistered) in reply to joaomc

    I'm not the only one!

  • (cs) in reply to Brian Reiter
    Brian Reiter:
    This is .NET, the author clearly should have used code access security attribributes instead of a password.

    [PrincipalPermission(SecurityAction.Demand, Role = "UpdateStatusFlippers")] OverrideIsUpdated( bool status ) { _IsUpdated = status; }

    I originally wanted to post the obvious "use md5()" comment. But I found that someone already posted that. And then somebody criticized it for not solving the problem of reverse engineering the caller. So I decided to post something like "use challenge and response, or hash password together with arguments". But I kept reading posts ... and found yours. It's great!:))) Of course it doesn't address the correct issue (we want to authorize the developer, not the user), but it made me laugh!:D
  • Johannes Hansen (unregistered) in reply to TheMagic8Ball

    Technically your right but that doesn't mean that using a password w. obfuscation solution would still be terrible! The correct way to secure that your code i only called by your own assembly is CAS. But still it should only be used in a few cases.

  • Peccadillo (unregistered) in reply to Miles
    Miles:
    ...apparently the coder was also unaware that C# can handle 'string' (lowercase) as if it were a primitive type.

    There is no difference in C# between the alias 'string' and the CLR type 'System.String'. They are both references to the same type; the C# compiler maps an object declared as a 'string' to a 'System.String' object.

    Furthermore, strings in C# are kind of a hybrid between value-types and reference-types ( value-type being the .NET parlance for a primitive ). For example, a string in .NET can be assigned the value null ( which is not possible for value-types ), but a string passed as an argument to a method is passed by value ( which is not typical for reference-types ).

  • Peccadillo (unregistered) in reply to Patrick
    Patrick:
    TheMagic8Ball:
    Technically, internal and private methods/properties can still be accessed using reflection. Assuming that they obfuscated the string, then this would guarentee that only someone who knew the password could call it.

    That doesn't mean it's a horrible example of code though ;-)

    Are you serious? By that same argument you could do something similar in C++ by taking a pointer to a class and modify its private members if you knew the structure of the object's layout in memory. It would rely on knowing a bit about the implementation of the compiler etc, but still possible. The access levels provided by the environment are more than sufficient in providing proper encapsulation of data. If somebody is using reflection to actually call code they're obviously doing something wrong.

    It's not quite that difficult to set a private field in a .NET class using Reflection. Here's an example of some code that creates a delegate that can do it:

            private static SetFieldValueMethod createSetFieldValueMethod( FieldInfo field )
            {
            Type[ ] parameterTypes = { field.DeclaringType, field.FieldType };
            DynamicMethod dynamicMethod = new DynamicMethod( string.Empty, typeof( void ), parameterTypes, field.DeclaringType );
            ILGenerator generator = dynamicMethod.GetILGenerator( );
            generator.Emit( OpCodes.Ldarg_0 );
            generator.Emit( OpCodes.Ldarg_1 );
            if( field.FieldType.IsClass )
                {
                generator.Emit( OpCodes.Castclass, field.FieldType );
                }
            else
                {
                generator.Emit( OpCodes.Unbox_Any, field.FieldType );
                }
            generator.Emit( OpCodes.Stfld, field );
            generator.Emit( OpCodes.Ret );
            return (SetFieldValueMethod) dynamicMethod.CreateDelegate( typeof( SetFieldValueMethod ) );
            }

Leave a comment on “Protected Code”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article