Tuesday, September 28, 2010

MS10 - 070 Post Mortem analysis of the patch

Now that Microsoft has patched the POET vulnerability, I thought it would be instructive to see what they changed. Now I'm not masochistic enough to disassemble aspnet_wp.exe or webengine.dll so there are things changed there that I don't know... but I did do a Reflector Export of the System.Web and System.Web.Extensions assemblies and then used BeyondCompare to do a diff against the files.

The analysis is simple:

  1. Don't leak exception information - This prevents exploits from seeing what is broken.
  2. Don't short-circuit on padding checks (take the same amount of time for padding correct verses padding broken) - This prevents exploits from seeing timing difference for incorrect padding.
  3. Don't be too picky about exception catching in IHttpHandler.ProcessRequest - This prevents exploits from seeing that you caught one kind of exception (CryptographicException) instead of all exceptions.
  4. Switch from Hash-based initialization vectors to Random IVs - This prevents exploits from using the relationship between the data and the hash to decrypt faster.
  5. Allow for backward compatibility - In case this breaks something, allow the new behavior to be reverted in-part.
  6. When doing a code review pass, change to make it clear you've considered the new options.

Here's the blow-by-blow review

Changes in System.Web

  • AssemblyInfo.cs
    (v2.0/v3.5) Bump AssemblyFileVersion from 2.0.50272.5014 to 2.0.50727.5053
    (v4.0) Bump AssemblyFileVersion from 4.0.30319.1 to 4.0.30319.206
  • ThisAssembly.cs
    (v2.0/v3.5) Bump InformationalVersion from 2.0.50272.5014 to 2.0.50727.5053
    (v4.0) Bump the BuildRevisionStr from 1 to 206
    (v4.) Bump InformationalVersion from 4.0.30319.1 to 4.0.30319.206
  • HttpCapabilitiesEvaluator.cs
    When caching the browser capabilities, sets the cache expiration to sliding time based on the the setting specified (instead of no sliding, no absolute)
  • MachineKeySection.cs
    Calls to EncryptOrDecryptData without specifying an initialization vector type now defaults to IVType.Random instead of IVType.Hash.
    Now uses AppSettings.UseLegacyEncryption flag to determine if the data should be signed.
    If decrypting and signing, now checks to ensure that the data has unhashed content (GetUnHashedData) and throws if there is no data other than the hash block. If there is no data after the signature, throws new Exception()!
    VerifyHashedData no longer fast-aborts when the hashed data mismatches. This makes the check take the same amount of time whether a match is found or not.
  • Handlers\AssemblyResourceLoader.cs
    ProcessRequest now catches any exception and morphs to an undistinguished InvalidRequest instead of leaking it out.
    (v4.0) Also no longer passes the assembly name to the InvalidRequest exception formatting.
  • Security\MachineKey.cs (v4.0)
    Decode respects the new AppSettings.UseLegacyMachineKeyEncryption setting. [note, not AppSettings.UseLegacyEncryption]
  • Security\MembershipAdapter.cs (v4.0)
    EncryptOrDecryptData now explicitly passes false for signData to MachineKeySection.EncryptOrDecryptData to remain compatible with prior calls, whilst still showing this code has been checked.
  • Security\MembershipProvider.cs
    DecryptPassword and EncryptPassword now explicitly passes false for useValidationSymAlgo and signData to MachineKeySection.EncryptOrDecryptData to remain compatible with prior calls, whilst still showing this code has been checked.
  • UI\WebControls CheckBoxField.cs CheckBoxList.cs MailDefinition.cs ObjectDataSource.cs and WebControl.cs
    (v4.0) Various property attributes have been reordered [unrelated, artifact of a new compiler version?]
  • UI\ObjectStateFormatter.cs
    Deserialize now ignores the caught exception and now throws the MacValidationError without leaking out the exception details.
  • UI\Page.cs
    Now flags the page response as in-error for any exception, not just a CryptographicException, thus not leaking out the exception kind.
  • Util\AppSettings.cs
    Now exposes a UseLegacyEncryption setting to allow reverting to old behavior for encryption. [defaults to false]
    (v4.0) Now exposes a UseLegacyMachineKeyEncryption setting to allow reverting to old behavior for MachineKey encryption. [defaults to false]
    (v4.0) Now exposes a ScriptResourceAllowNonJsFiles setting to allow reverting to old behavior for ScriptResource encryption. [defaults to false]
  • Util\VersionInfo.cs (v4.0)
    Bumped the serialized SystemWebVersion property from 4.0.30319.1 to 4.0.30319.206

System.Web.Extensions (v4.0)

  • ScriptResourceHandler.cs
    ProcessRequest now catches any exception and throws an undistinguished 404 error.
    ProcessRequest now uses the new AppSettings.ScriptResourceAllowNonJsFiles setting to control behavior when a resource is requested that doesn't end in ".js" If enabled (NOT DEFAULT), then resource is allowed, otherwise now will throw an undistinguished 404 error.
  • VersionInfo.cs
    Bumped the serialized SystemWebVersion property from 4.0.30319.1 to 4.0.30319.206
  • ThisAssembly.cs
    Bumped the build revision from 1 to 206