• JK (unregistered)

    Frist!

  • Second (unregistered)

    Not frist

  • Thridly speaking (unregistered)

    plus 1 point for optimisation, minus 100 points for a race condition

  • MiserableOldGit (unregistered)

    Leaving aside the very misleading name, was it supposed to be a method for sanitising dirty inputs, or is TRWTF (in terms of the CVE) that a step is being skipped?

    And I've not done much SeeHash, but it looks to me like this is replacing any instance of a double quote with two double quotes, someone educate me!

  • rabble (unregistered) in reply to MiserableOldGit

    rabble rabble

  • Bas (unregistered)

    Actually the string is valid: @" DERP DERP HALLO WORLD" is a valid string in C# The @ marks it as a special case that automatically esacapes a lot of stuff.

  • Sole Purpose of VIsit (unregistered) in reply to Bas

    Technically, it's a "verbatim string literal." With only a single exception, you don't need to escape any character in the string.

    Exercise for the reader: which character is the exception, and how do you escape it? (Both answers are obvious, if you think about it.)

  • Nobody Important (unregistered)

    I swear to god, the last competent programmer left microsoft many years ago - all they've got left is incompetent buffoons trying to keep stuff working as best they can. And don't even get me started on their UX.

  • MiserableOldGit (unregistered) in reply to Sole Purpose of VIsit
    Exercise for the reader: which character is the exception, and how do you escape it? (Both answers are obvious, if you think about it.)

    Presumably that also answers my question .... getting confused between it being escaped with " or "". As the vuln Remy points to only occurs if you stick more than one address in the location, because I assume getting an unescaped double quote through IsValidURL would not be possible (?), then I guess the other major WTF is why does the code calling the WDSL parser bother taking those additional addresses and adding them as comments? That seems like asking for trouble.

  • MiserableOldGit (unregistered)

    and there was a backslash in my post which I didn't escape, I'm sure you know where. I did it for ironic reasons, honest.

  • Foo AKA Fooo (unregistered)

    "I’m willing to grant that re-using the same static StringBuilder object is a performance tuning thing". -- I'm not. This literally screams premature optimization and race condition. How much there's another exploit lurking there?

    2 serious exploits in ~20 lines of code, a bit more than mildly embarassing, I'd say.

    (PS: Probably some else posted the same meanwhile. Many of you, hopefully. The delay in this post is, again, due to reCAPTCHA. When I clicked to hear the audio version (which is not quite as broken as the images), it claimed I was sending automated requests. So clicking one icon in the way it's supposed to be used, not doing anything else, now makes you a robot? Meanwhile the real spammers have no problem getting through. Really, fuck reCAPTCHA! It's worse than useless.)

  • Jerepp (unregistered) in reply to Foo AKA Fooo

    So you are saying somebody invented a robot to click the I'm not a robot button?

  • Sole Purpose of VIsit (unregistered) in reply to MiserableOldGit

    To be honest? I don't have a clue about the "asking for trouble" bit. I'm typing this on a Linux box, so I can't play around with the OP drivel code in VS right now. (Yes, you're right -- the answer, in both cases, is of course the double quote character.)

    This is a fairly impressively nasty bit of code: it uses four hard-coded strings, one of which is repeated for no obvious reason, one of which is an escaped string used as a single character (as it were -- single quotes), and one of which is the said verbatim string literal. It's almost as if the programmer responsible wanted to cause brain implosion on the poor maintenance sods who came after.

    Then again, this is apparently WSDL. WSDL is like that. It's one of the more brain-damaged protocols out there ... and it doesn't really surprise me that an accidental oopsie in WSDL parsing opens up zero-day defects in simple things like RTF apps.

  • Sole Purpose of VIsit (unregistered)

    And, of course, despite Remy's apologia pro bugga otro, the concept of returning a static stringbuilder instantiation is pure insanity.

    Given the default way that static data is handled by the .NET VM, it isn't even particularly efficient.

  • eric bloedow (unregistered)

    this reminds me of an old story about a program called "sendmail". it was used to send messages from user to user over a network. BUT...a bug in the program allowed it to send messages to ANY part of the network, including SYSTEM folders.! hackers could use it to "mail" fake user profiles into the system, thus acquiring admin powers! the company DID some up with a fix, but many sysops were too lazy or sloppy to apply the fix...

  • Carl Witthoft (google)

    I read a "string article" once. The physicists who wrote it were completely out of their branes.

  • C# dev (unregistered)

    Not sure why he's generating a verbatim string literal like that, but the comment about new lines is incorrect. For verbatim string literals (the ones with an @ before the first quote), you don't need to comment out new lines. In C# var text = @"this is a text"; is perfectly valid.

  • MiserableOldGit (unregistered) in reply to Sole Purpose of VIsit

    Sorry, you have to root about a bit ... Remy didn't make reference to this in the article directly. IsValidURL is called from here ... maybe other places too, but this is the one identified by FireEye. I just can't see the point in sending through the additional addresses, and doing so has opened up this code injection vuln. OK, it was there to be opened, but you could argue IsValidURL shouldn't be being used to reformat a comment.

    Although it is just a happy accident it isn't also vulnerable for the initial address. Unless there's someway you can shove a backspace through that string literal, in which case, bombs away!

                if (_connectURLs != null)
            {
                for (int i=0; i<_connectURLs.Count; i++)
                {
                    sb.Length = 0;
                    sb.Append(indent2);
                    if (i == 0)
                    {
                        sb.Append("base.ConfigureProxy(this.GetType(), ");
                        sb.Append(WsdlParser.IsValidUrl((string)_connectURLs[i]));
                        sb.Append(");");
                    }
                    else
                    {
                        // Only the first location is used, the rest are commented out in the proxy
                        sb.Append("//base.ConfigureProxy(this.GetType(), ");
                        sb.Append(WsdlParser.IsValidUrl((string)_connectURLs[i]));
                        sb.Append(");");
                    }
                    textWriter.WriteLine(sb);
                }
            }
  • mgm (unregistered)

    What is the name of the author of this code? Rahul, Abhishek, Aditya, Amit?

  • MiserableOldGit (unregistered) in reply to mgm

    His name is in the boilerplate, and it looks far more french or dutch to me, so I guess you'll have to save the casual racism for another article!

  • Alex (unregistered) in reply to Nobody Important

    Hey, @Nobody Important! I only quit last year.

  • herby (unregistered)

    This is but one instance that appears to confirm my belief that anything called "Windows" is a virus that is infecting computers and must be eradicated (sooner the better).

  • John A (unregistered) in reply to Nobody Important

    Probably the same programmer that did the Zune is responsible here.

  • Sole Purpose of VIsit (unregistered) in reply to herby

    Best of luck in pursuing a professional career then, Herby. Employers just love that sort of attitude.

  • Sole Purpose of VIsit (unregistered) in reply to MiserableOldGit

    Well, it's all very exciting, innit? For Your Eyes Only: this is CVE-2017-8759. And, although I would be the last person to accuse the OP submitter of jumping on a fairly pointless band-waggon and taking a cheap and largely pointless shot at Microsoft, it's basically an "exploit" announced on September 12th by FireEye.

    Thing is, it's not all that exciting at all. It's ugly and broken code, sure, but it's no different (as far as I can see) to any other "exploit" embedded in a document attached to an email that begs you to open it as Admin.

    I'm wide open to being proven wrong here. Feel free to prove me wrong.

  • Sole Purpose of VIsit (unregistered)

    "The real WTF is the fact that you can embed SOAP links in RTF files and Word will attempt to use them, thus running the WSDL parser against the input data. This is code that’s a little bad, used badly, creating an exploited zero-day."

    Silly sort of a question, I suppose, Remy, but is this just a pile of bollocks, or can you back it up?

  • Cat (unregistered)

    The code posted has no bug with newlines. It will return newlines in a verbatim string literal, which is entirely valid, and in fact is the only way to include a newline in a verbatim literal.

    The exploit came about because the caller of this code assumed the resultant string would not contain an embedded newline, and thus assumed that prefixing the return value with // would comment out the entire string, rather than just the first line of a multi-line verbatim string.

  • (nodebb)

    I thought code from open source projects didn't qualify for this site?

  • doubting_poster (unregistered)

    this was a proper wtf. refreshing!

  • Paul Neumann (unregistered)

    Always indicative of quality code:

                public override int GetHashCode()
            {
                return base.GetHashCode();
            }
  • jgh (unregistered)

    You can get trapped even when English is your first language. Just yesterday I had a lightbulb moment when I realised the dead ends I was scurrying down was all because I'd labelled a set of routines 'Generic' when I should have labelled them 'Universal'.

  • Zenith (unregistered) in reply to Nobody Important

    What's really scary is reading through the Windows Forms namespace. I read through that pretty regularly when I was developing a control library. There are really times where dropping down to Win32 and starting over seemed like a smarter choice that developing on top of that.

  • BernieTheBernie (unregistered) in reply to Paul Neumann

    Another indication of the high code quality is the fact that functions end with a return statement. Even when they are declared void.

    By the way, the code seems to have multiple authors: apart from the functions not ending with a return statement, the copyright statement on top of it shows a date from 2000, but somewhere further down autoproperties are used which became available with .Net 4 only. Poor guy who added his name there - he could be mistaken to be the author of the abominations in this code.

    Another snippet I really like:

                  if (soapOp.style == "rpc")
                    bRpcBinding = true;
                {
                }
  • (nodebb)

    Outside the obvious WTF of having Word parse SOAP links in RTF files, the main problem here isn't the C# verbatim strings themselves, nor the function to make them (besides its non-indicative name). The problem is that the programmer ignored the difference between C# verbatim strings and C# line comments: The former accepting newlines unescaped, whereas the latter requires escaping (or inserting comment markup at the start of each line).

  • Patrick (unregistered)

    Forgetting to escape newline is a real classic. Like the ancient phf vulnerability. It was a CGI script (included by default with some HTTP server for Solaris IIRC). You invoked /cgi-bin/phf?Qalias=blah and it passed 'blah' as argument to some executable using system(). Well, it escaped most shell metacharacters... except newline. So a simple request to /cgi-bin/phf?Qalias=%0acmd was enough to run any command of your choice as the httpd user...

  • Peter (unregistered) in reply to Patrick

    It is C#, not C. No need to escape a newline in a verbatim string.

  • EIS (unregistered)

    Usually, when I read MSFT code I feel good about my .net code after, really good. That entire class is totally violating SOLID principles, no DI, messy naming scheme for variables, hard to understand methods, too long methods with so much complexity I feel like the class was written in .Net 1.1 10 years ago by a junior developer.

  • moose (unregistered)

    Go look at how Python generates namedtuples... it's just amazing. https://github.com/python/cpython/blob/2.7/Lib/collections.py#L247-L400

Leave a comment on “string isValidArticle(string article)”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article