[1.4>master] [MERGE #2427 @Cellule] WASM: section id read past buffer

Merge pull request #2427 from Cellule:users/micfer/wasm/section_id

lib/WasmReader/WasmBinaryReader.cpp

@@ -65,7 +65,7 @@ WasmBinaryReader::WasmBinaryReader(ArenaAllocator* alloc, Js::WebAssemblyModule
 {
     m_start = m_pc = source;
     m_end = source + length;
-    m_currentSection.code = bSectInvalid;
+    m_currentSection.code = bSectLimit;
 #if DBG_DUMP
     m_ops = Anew(m_alloc, OpSet, m_alloc);
 #endif
@@ -81,7 +81,7 @@ void WasmBinaryReader::InitializeReader()
         const byte* startModule = m_pc;
 
         bool doRead = true;
-        SectionCode prevSect = bSectInvalid;
+        SectionCode prevSect = bSectLimit;
         while (doRead)
         {
             SectionHeader secHeader = ReadSectionHeader();
@@ -156,7 +156,7 @@ WasmBinaryReader::ReadNextSection(SectionCode nextSection)
 bool
 WasmBinaryReader::ProcessCurrentSection()
 {
-    Assert(m_currentSection.code != bSectInvalid);
+    Assert(m_currentSection.code != bSectLimit);
     TRACE_WASM_SECTION(_u("Process section %s"), SectionInfo::All[m_currentSection.code].name);
     m_readerState = READER_STATE_MODULE;
 
@@ -217,38 +217,30 @@ WasmBinaryReader::ReadSectionHeader()
 {
     SectionHeader header;
     header.start = m_pc;
-    header.code = bSectInvalid;
+    header.code = bSectLimit;
 
     UINT len = 0;
-    UINT32 sectionId = LEB128(len);
+    CompileAssert(sizeof(SectionCode) == sizeof(uint8));
+    SectionCode sectionId = (SectionCode)ReadVarUInt7();
+
+    if (sectionId > bsectLastKnownSection)
+    {
+        ThrowDecodingError(_u("Invalid known section opcode %u"), sectionId);
+    }
 
     UINT32 sectionSize = LEB128(len);
     header.end = m_pc + sectionSize;
     CheckBytesLeft(sectionSize);
 
-    const char *sectionName = nullptr;
-    UINT32 nameLength = 0;
-
-    if (sectionId > 0)
-    {
-        SectionCode sectCode = (SectionCode)(sectionId - 1);
-
-        if (sectCode >= bSectNames) // ">=" since "Name" isn't considered to be a known section
-        {
-            ThrowDecodingError(_u("Invalid known section opcode %d"), sectCode);
-        }
-
-        sectionName = SectionInfo::All[sectCode].id;
-        header.code = sectCode;
-        nameLength = static_cast<UINT32>(strlen(sectionName)); //sectionName (SectionInfo.id) is null-terminated
-    }
-    else
+    header.code = sectionId;
+    const char *sectionName = SectionInfo::All[sectionId].id;
+    UINT32 nameLength = SectionInfo::All[sectionId].nameLength;
+    if (sectionId == bSectCustom)
     {
         nameLength = LEB128(len);
         CheckBytesLeft(nameLength);
         sectionName = (const char*)(m_pc);
         m_pc += nameLength;
-        header.code = bSectCustom;
     }
 
     header.nameLength = nameLength;
@@ -1106,11 +1098,9 @@ WasmBinaryReader::LEB128(UINT &length, bool sgn)
     uint maxReads = sizeof(MaxAllowedType) == 4 ? 5 : 10;
     CompileAssert(sizeof(MaxAllowedType) == 4 || sizeof(MaxAllowedType) == 8);
 
-    // LEB128 needs at least one byte
-    CheckBytesLeft(1);
-
     for (uint i = 0; i < maxReads; i++, length++)
     {
+        CheckBytesLeft(1);
         b = *m_pc++;
         result = result | ((MaxAllowedType)(b & 0x7f) << shamt);
         if (sgn)
@@ -1238,6 +1228,12 @@ T WasmBinaryReader::ReadConst()
     return value;
 }
 
+uint8
+WasmBinaryReader::ReadVarUInt7()
+{
+    return ReadConst<uint8>() & 0x7F;
+}
+
 bool WasmBinaryReader::ReadMutableValue()
 {
     uint8 mutableValue = ReadConst<UINT8>();

lib/WasmReader/WasmBinaryReader.h

@@ -83,6 +83,7 @@ namespace Wasm
         // Primitive reader
         template <WasmTypes::WasmType type> void ConstNode();
         template <typename T> T ReadConst();
+        uint8 ReadVarUInt7();
         bool ReadMutableValue();
         const char16* ReadInlineName(uint32& length, uint32& nameLength);
         const char16* CvtUtf8Str(LPCUTF8 name, uint32 nameLen, charcount_t* dstLength = nullptr);

lib/WasmReader/WasmByteCodeGenerator.cpp

@@ -86,12 +86,12 @@ WasmModuleGenerator::GenerateModule()
 
     BVStatic<bSectLimit + 1> visitedSections;
 
-    for (SectionCode sectionCode = (SectionCode)(bSectInvalid + 1); sectionCode < bSectLimit ; sectionCode = (SectionCode)(sectionCode + 1))
+    for (uint8 sectionCode = bSectCustom + 1; sectionCode < bSectLimit ; ++sectionCode)
     {
         SectionCode precedent = SectionInfo::All[sectionCode].precedent;
         if (GetReader()->ReadNextSection((SectionCode)sectionCode))
         {
-            if (precedent != bSectInvalid && !visitedSections.Test(precedent))
+            if (precedent != bSectLimit && !visitedSections.Test(precedent))
             {
                 throw WasmCompilationException(_u("%s section missing before %s"),
                                                SectionInfo::All[precedent].name,

lib/WasmReader/WasmSection.cpp

@@ -9,7 +9,7 @@
 namespace Wasm
 {
     SectionInfo SectionInfo::All[bSectLimit] = {
-#define WASM_SECTION(name, id, flag, precedent) {flag, bSect ## precedent, static_cast<const char16*>(_u(#name)), static_cast<const char*>(id)},
+#define WASM_SECTION(name, id, flag, precedent) {flag, bSect ## precedent, static_cast<const char16*>(_u(#name)), static_cast<const char*>(id), sizeof(#name)},
 #include "WasmSections.h"
     };
 }

lib/WasmReader/WasmSection.h

@@ -16,11 +16,11 @@ namespace Wasm
     };
 
 #define WASM_SECTION(name, id, flag, precendent) bSect ## name,
-    enum SectionCode
+    enum SectionCode : uint8
     {
-        bSectInvalid = -1,
 #include "WasmSections.h"
-        bSectLimit
+        bSectLimit,
+        bsectLastKnownSection = bSectDataSegments
     };
 
     struct SectionInfo
@@ -29,6 +29,7 @@ namespace Wasm
         SectionCode precedent;
         const char16* name;
         const char* id;
+        const uint32 nameLength;
         static SectionInfo All[bSectLimit];
     };
 }

lib/WasmReader/WasmSections.h

@@ -4,17 +4,19 @@
 //-------------------------------------------------------------------------------------------------------
 
 //          (name                 , ID                   , SectionFlag, Precedent         )
-WASM_SECTION(Signatures           , "type"               , fSectNone  , Invalid           )
-WASM_SECTION(ImportTable          , "import"             , fSectNone  , Invalid           )
+WASM_SECTION(Custom               , ""                   , fSectNone  , Limit             )
+WASM_SECTION(Signatures           , "type"               , fSectNone  , Limit             )
+WASM_SECTION(ImportTable          , "import"             , fSectNone  , Limit             )
 WASM_SECTION(FunctionSignatures   , "function"           , fSectNone  , Signatures        )
-WASM_SECTION(IndirectFunctionTable, "table"              , fSectNone  , Invalid           )
-WASM_SECTION(Memory               , "memory"             , fSectNone  , Invalid           )
-WASM_SECTION(Global               , "global"             , fSectNone  , Invalid           )
-WASM_SECTION(ExportTable          , "export"             , fSectNone  , Invalid           )
+WASM_SECTION(IndirectFunctionTable, "table"              , fSectNone  , Limit             )
+WASM_SECTION(Memory               , "memory"             , fSectNone  , Limit             )
+WASM_SECTION(Global               , "global"             , fSectNone  , Limit             )
+WASM_SECTION(ExportTable          , "export"             , fSectNone  , Limit             )
 WASM_SECTION(StartFunction        , "start"              , fSectNone  , Signatures        )
-WASM_SECTION(Element              , "element"            , fSectNone  , Invalid           )
+WASM_SECTION(Element              , "element"            , fSectNone  , Limit             )
 WASM_SECTION(FunctionBodies       , "code"               , fSectNone  , FunctionSignatures)
-WASM_SECTION(DataSegments         , "data"               , fSectNone  , Invalid           )
+WASM_SECTION(DataSegments         , "data"               , fSectNone  , Limit             )
 WASM_SECTION(Names                , "name"               , fSectIgnore, Signatures        )
-WASM_SECTION(Custom               , ""                   , fSectNone  , Invalid           )
+// Check for custom sections at the end as well
+WASM_SECTION(Custom2              , ""                   , fSectNone  , Limit             )
 #undef WASM_SECTION

test/wasm/baselines/api.baseline

@@ -21,7 +21,7 @@ Test 4 passed. Expected Error: TypeError: BufferSource expected
 Test 5 passed. Expected Error: TypeError: BufferSource expected
 Test 6 passed. Expected Error: TypeError: BufferSource expected
 Test 7 passed. Expected Error: TypeError: BufferSource expected
-Test 8 passed. Expected Error: WebAssemblyCompileError: Invalid LEB128 format
+Test 8 passed. Expected Error: WebAssemblyCompileError: Out of file: Needed: 1, Left: 0
 Test 9 passed. Expected Error: TypeError: BufferSource expected
 Testing module
 exports
@@ -38,7 +38,7 @@ Test 4 passed. Expected Error: TypeError: BufferSource expected
 Test 5 passed. Expected Error: TypeError: BufferSource expected
 Test 6 passed. Expected Error: TypeError: BufferSource expected
 Test 7 passed. Expected Error: TypeError: BufferSource expected
-Test 8 passed. Expected Error: WebAssemblyCompileError: Invalid LEB128 format
+Test 8 passed. Expected Error: WebAssemblyCompileError: Out of file: Needed: 1, Left: 0
 Test 9 passed. Expected Error: TypeError: BufferSource expected
 Test 10 passed. Expected Error: TypeError: Object expected
 Test 11 passed. Expected Error: TypeError: Object expected
@@ -86,7 +86,7 @@ Test 6 passed. Expected Error: TypeError: BufferSource expected
 Test 7 passed. Expected Error: TypeError: BufferSource expected
 Test 8 passed. Expected Error: TypeError: BufferSource expected
 Test 9 passed. Expected Error: TypeError: BufferSource expected
-Test 10 passed. Expected Error: WebAssemblyCompileError: Invalid LEB128 format
+Test 10 passed. Expected Error: WebAssemblyCompileError: Out of file: Needed: 1, Left: 0
 Test 11 passed. Expected Error: TypeError: BufferSource expected
 Testing module
 exports

test/wasm/bugs.js

@@ -3,9 +3,34 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 WScript.Flag("-wasmI64");
+function createView(bytes) {
+  const buffer = new ArrayBuffer(bytes.length);
+  const view = new Uint8Array(buffer);
+  for (let i = 0; i < bytes.length; ++i) {
+    view[i] = bytes.charCodeAt(i);
+  }
+  return view;
+}
 async function main() {
   const {instance: {exports: {foo}}} = await WebAssembly.instantiate(readbuffer("binaries/bug_fitsdword.wasm"));
   foo();
+
+  try {
+    new WebAssembly.Module(createView(`\x00asm\x0d\x00\x00\x00\xff\xff\xff\xff\x7f\x00\x00\x00`));
+    console.log("Should have had an error");
+  } catch (e) {
+    if (!(e instanceof WebAssembly.CompileError)) {
+      throw e;
+    }
+  }
+  try {
+    new WebAssembly.Module(createView(`\x00asm\x0d\x00\x00\x00\x7f\x00\x00\x00`));
+    console.log("Should have had an error");
+  } catch (e) {
+    if (!(e instanceof WebAssembly.CompileError)) {
+      throw e;
+    }
+  }
 }
 
 main().then(() => console.log("PASSED"), console.log);