fix: rework FeatureExt generation (#1718)

Author: arthurschreiber

src/login7-payload.ts

@@ -251,16 +251,24 @@ class Login7Payload {
     }
 
     // (ibUnused / ibExtension): 2-byte
+    // For TDS 7.4+, this points to a 4-byte offset (ibFeatureExtLong) in the data section.
+    // The actual FeatureExt data is placed at the END of the packet per MS-TDS spec.
     offset = fixedData.writeUInt16LE(dataOffset, offset);
 
     // (cchUnused / cbExtension): 2-byte
+    // For TDS 7.4+, this is the size of the ibFeatureExtLong offset pointer (4 bytes).
+    // The actual FeatureExt data is appended at the end of the packet, not here.
+    // We'll store the FeatureExt data to append at the end after all other variable data.
+    let featureExtData: Buffer | undefined;
+    let extensionOffsetBuffer: Buffer | undefined;
     if (this.tdsVersion >= versions['7_4']) {
-      const extensions = this.buildFeatureExt();
-      offset = fixedData.writeUInt16LE(4 + extensions.length, offset);
-      const extensionOffset = Buffer.alloc(4);
-      extensionOffset.writeUInt32LE(dataOffset + 4, 0);
-      dataOffset += 4 + extensions.length;
-      buffers.push(extensionOffset, extensions);
+      featureExtData = this.buildFeatureExt();
+      // cbExtension = 4 (size of the ibFeatureExtLong pointer, not the FeatureExt data)
+      offset = fixedData.writeUInt16LE(4, offset);
+      // Reserve space for the 4-byte offset pointer; we'll fill in the actual offset later
+      extensionOffsetBuffer = Buffer.alloc(4);
+      buffers.push(extensionOffsetBuffer);
+      dataOffset += 4;
     } else {
       // For TDS < 7.4, these are unused fields
       offset = fixedData.writeUInt16LE(0, offset);
@@ -329,6 +337,7 @@ class Login7Payload {
       }
 
       buffers.push(this.sspi);
+      dataOffset += this.sspi.length;
     } else {
       offset = fixedData.writeUInt16LE(0, offset);
     }
@@ -370,6 +379,15 @@ class Login7Payload {
       fixedData.writeUInt32LE(0, offset);
     }
 
+    // Per MS-TDS spec, FeatureExt data must be at the END of the packet,
+    // after all other variable-length data.
+    if (featureExtData && extensionOffsetBuffer) {
+      // Update the ibFeatureExtLong offset to point to where FeatureExt will be
+      extensionOffsetBuffer.writeUInt32LE(dataOffset, 0);
+      // Append FeatureExt data at the end
+      buffers.push(featureExtData);
+    }
+
     const data = Buffer.concat(buffers);
     data.writeUInt32LE(data.length, 0);
     return data;

test/unit/login7-payload-test.ts

@@ -246,8 +246,7 @@ describe('Login7Payload', function() {
           connectionId: 0,
           clientTimeZone: 120,
           clientLcid: 0x00000409,
-          isFabric: true,
-        } as any);
+        });
 
         payload.hostname = 'example.com';
         payload.appName = 'app';
@@ -288,5 +287,170 @@ describe('Login7Payload', function() {
         assert.lengthOf(data, expectedLength);
       });
     });
+
+    describe('FeatureExt positioning per MS-TDS spec', function() {
+      it('places ibFeatureExtLong offset pointer at the correct position', function() {
+        const payload = new Login7Payload({
+          tdsVersion: 0x74000004,
+          packetSize: 1024,
+          clientProgVer: 0,
+          clientPid: 12345,
+          connectionId: 0,
+          clientTimeZone: 120,
+          clientLcid: 0x00000409,
+        });
+
+        payload.hostname = 'testhost';
+        payload.userName = 'testuser';
+        payload.password = 'testpass';
+        payload.appName = 'TestApp';
+        payload.serverName = 'testserver';
+        payload.language = 'us_english';
+        payload.database = 'master';
+        payload.libraryName = 'Tedious';
+        payload.fedAuth = {
+          type: 'ADAL',
+          echo: true,
+          workflow: 'default'
+        };
+
+        const data = payload.toBuffer();
+
+        // ibExtension is at fixed offset 56 (2 bytes for offset, 2 bytes for length)
+        const ibExtension = data.readUInt16LE(56);
+        const cbExtension = data.readUInt16LE(58);
+
+        // cbExtension should be 4 (the size of the ibFeatureExtLong pointer)
+        assert.strictEqual(cbExtension, 4, 'cbExtension should be 4 bytes (ibFeatureExtLong pointer size)');
+
+        // Read the ibFeatureExtLong value (4-byte offset to FeatureExt data)
+        const ibFeatureExtLong = data.readUInt32LE(ibExtension);
+
+        // Verify ibFeatureExtLong points to a valid position in the packet
+        assert.isAtLeast(ibFeatureExtLong, 94, 'ibFeatureExtLong should point past the fixed header');
+        assert.isBelow(ibFeatureExtLong, data.length, 'ibFeatureExtLong should point within the packet');
+
+        // Verify FeatureExt data starts at the position pointed to by ibFeatureExtLong
+        // First feature should be FEDAUTH (0x02) or UTF8_SUPPORT (0x0A)
+        const firstFeatureId = data.readUInt8(ibFeatureExtLong);
+        assert.oneOf(firstFeatureId, [0x02, 0x0A], 'First feature should be FEDAUTH (0x02) or UTF8_SUPPORT (0x0A)');
+
+        // Find all variable data fields to verify FeatureExt is at the END
+        // Fixed header layout (after ClientLCID at offset 32-35):
+        // 36-39: ibHostName/cchHostName, 40-43: ibUserName/cchUserName
+        // 44-47: ibPassword/cchPassword, 48-51: ibAppName/cchAppName
+        // 52-55: ibServerName/cchServerName, 56-59: ibExtension/cbExtension
+        // 60-63: ibCltIntName/cchCltIntName, 64-67: ibLanguage/cchLanguage
+        // 68-71: ibDatabase/cchDatabase, 72-77: ClientID (6 bytes)
+        // 78-81: ibSSPI/cbSSPI, 82-85: ibAtchDBFile/cchAtchDBFile
+        // 86-89: ibChangePassword/cchChangePassword, 90-93: cbSSPILong
+        const ibHostName = data.readUInt16LE(36);
+        const cchHostName = data.readUInt16LE(38);
+        const ibUserName = data.readUInt16LE(40);
+        const cchUserName = data.readUInt16LE(42);
+        const ibPassword = data.readUInt16LE(44);
+        const cchPassword = data.readUInt16LE(46);
+        const ibAppName = data.readUInt16LE(48);
+        const cchAppName = data.readUInt16LE(50);
+        const ibServerName = data.readUInt16LE(52);
+        const cchServerName = data.readUInt16LE(54);
+        const ibCltIntName = data.readUInt16LE(60);
+        const cchCltIntName = data.readUInt16LE(62);
+        const ibLanguage = data.readUInt16LE(64);
+        const cchLanguage = data.readUInt16LE(66);
+        const ibDatabase = data.readUInt16LE(68);
+        const cchDatabase = data.readUInt16LE(70);
+        const ibSSPI = data.readUInt16LE(78);
+        const cbSSPI = data.readUInt16LE(80);
+        const ibAttachDBFile = data.readUInt16LE(82);
+        const cchAttachDBFile = data.readUInt16LE(84);
+        const ibChangePassword = data.readUInt16LE(86);
+        const cchChangePassword = data.readUInt16LE(88);
+
+        // Calculate the end of all regular variable data (excluding FeatureExt)
+        // Include all variable-length fields to ensure complete coverage
+        const variableDataEnd = Math.max(
+          ibHostName + cchHostName * 2,
+          ibUserName + cchUserName * 2,
+          ibPassword + cchPassword * 2,
+          ibAppName + cchAppName * 2,
+          ibServerName + cchServerName * 2,
+          ibCltIntName + cchCltIntName * 2,
+          ibLanguage + cchLanguage * 2,
+          ibDatabase + cchDatabase * 2,
+          ibSSPI + cbSSPI,
+          ibAttachDBFile + cchAttachDBFile * 2,
+          ibChangePassword + cchChangePassword * 2
+        );
+
+        // FeatureExt should start after all other variable data
+        assert.isAtLeast(ibFeatureExtLong, variableDataEnd,
+                         'FeatureExt (at ' + ibFeatureExtLong + ') should be after all variable data (ends at ' + variableDataEnd + ')');
+
+        // Verify FeatureExt ends with terminator (0xFF)
+        // Find the terminator by scanning from ibFeatureExtLong
+        let featureOffset = ibFeatureExtLong;
+        while (featureOffset < data.length) {
+          const featureId = data.readUInt8(featureOffset);
+          if (featureId === 0xFF) {
+            // Found terminator, verify it's at the end of the packet
+            assert.strictEqual(featureOffset, data.length - 1,
+                               'FeatureExt terminator should be the last byte of the packet');
+            break;
+          }
+          // Skip past this feature: 1 byte ID + 4 bytes length + length bytes data
+          const featureLen = data.readUInt32LE(featureOffset + 1);
+          featureOffset += 1 + 4 + featureLen;
+        }
+      });
+
+      it('correctly positions FeatureExt when SSPI data is present', function() {
+        const payload = new Login7Payload({
+          tdsVersion: 0x74000004,
+          packetSize: 1024,
+          clientProgVer: 0,
+          clientPid: 12345,
+          connectionId: 0,
+          clientTimeZone: 120,
+          clientLcid: 0x00000409,
+        });
+
+        payload.hostname = 'testhost';
+        payload.appName = 'TestApp';
+        payload.serverName = 'testserver';
+        payload.language = 'us_english';
+        payload.database = 'master';
+        payload.libraryName = 'Tedious';
+        // Add SSPI data to test offset calculation
+        payload.sspi = Buffer.from([0x4E, 0x54, 0x4C, 0x4D, 0x53, 0x53, 0x50, 0x00]); // "NTLMSSP\0"
+        payload.fedAuth = {
+          type: 'ADAL',
+          echo: true,
+          workflow: 'default'
+        };
+
+        const data = payload.toBuffer();
+
+        // Read the ibExtension and ibFeatureExtLong
+        const ibExtension = data.readUInt16LE(56);
+        const ibFeatureExtLong = data.readUInt32LE(ibExtension);
+
+        // ibSSPI is at offset 78, cbSSPI at offset 80 (after ClientID at 72-77)
+        const ibSSPI = data.readUInt16LE(78);
+        const cbSSPI = data.readUInt16LE(80);
+
+        // SSPI data should be present in the packet
+        assert.strictEqual(cbSSPI, 8, 'SSPI length should be 8 bytes');
+
+        // FeatureExt should be after SSPI data
+        const sspiEnd = ibSSPI + cbSSPI;
+        assert.isAtLeast(ibFeatureExtLong, sspiEnd,
+                         'FeatureExt should be after SSPI data');
+
+        // Verify first feature is valid
+        const firstFeatureId = data.readUInt8(ibFeatureExtLong);
+        assert.oneOf(firstFeatureId, [0x02, 0x0A], 'First feature should be FEDAUTH (0x02) or UTF8_SUPPORT (0x0A)');
+      });
+    });
   });
 });