Merge pull request #2397 from nspcc-dev/is-script-correct-panic

vm: avoid panic in `IsScriptCorrect`
This commit is contained in:
Roman Khimov 2022-03-18 10:08:39 +03:00 committed by GitHub
commit c2845852ae
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 36 additions and 6 deletions

View file

@ -160,23 +160,31 @@ func IsScriptCorrect(script []byte, methods bitfield.Field) error {
opcode.JMPIFNOTL, opcode.JMPEQL, opcode.JMPNEL, opcode.JMPIFNOTL, opcode.JMPEQL, opcode.JMPNEL,
opcode.JMPGTL, opcode.JMPGEL, opcode.JMPLTL, opcode.JMPLEL, opcode.JMPGTL, opcode.JMPGEL, opcode.JMPLTL, opcode.JMPLEL,
opcode.ENDTRYL, opcode.CALLL, opcode.PUSHA: opcode.ENDTRYL, opcode.CALLL, opcode.PUSHA:
off, _, err := calcJumpOffset(ctx, param) // It does bounds checking. off, _, err := calcJumpOffset(ctx, param)
if err != nil { if err != nil {
return err return err
} }
jumps.Set(off) // `calcJumpOffset` does bounds checking but can return `len(script)`.
// This check avoids panic in bitset when script length is a multiple of 64.
if off != len(script) {
jumps.Set(off)
}
case opcode.TRY, opcode.TRYL: case opcode.TRY, opcode.TRYL:
catchP, finallyP := getTryParams(op, param) catchP, finallyP := getTryParams(op, param)
off, _, err := calcJumpOffset(ctx, catchP) off, _, err := calcJumpOffset(ctx, catchP)
if err != nil { if err != nil {
return err return err
} }
jumps.Set(off) if off != len(script) {
jumps.Set(off)
}
off, _, err = calcJumpOffset(ctx, finallyP) off, _, err = calcJumpOffset(ctx, finallyP)
if err != nil { if err != nil {
return err return err
} }
jumps.Set(off) if off != len(script) {
jumps.Set(off)
}
case opcode.NEWARRAYT, opcode.ISTYPE, opcode.CONVERT: case opcode.NEWARRAYT, opcode.ISTYPE, opcode.CONVERT:
typ := stackitem.Type(param[0]) typ := stackitem.Type(param[0])
if !typ.IsValid() { if !typ.IsValid() {

View file

@ -191,13 +191,20 @@ func TestIsScriptCorrect(t *testing.T) {
t.Run("out of bounds JMPL 1", func(t *testing.T) { t.Run("out of bounds JMPL 1", func(t *testing.T) {
bad := getScript() bad := getScript()
bad[jmplOff+1] = byte(len(bad) - jmplOff) bad[jmplOff+1] = byte(len(bad)-jmplOff) + 1
bad[jmplOff+2] = 0 bad[jmplOff+2] = 0
bad[jmplOff+3] = 0 bad[jmplOff+3] = 0
bad[jmplOff+4] = 0 bad[jmplOff+4] = 0
require.Error(t, IsScriptCorrect(bad, nil)) require.Error(t, IsScriptCorrect(bad, nil))
}) })
t.Run("JMP to a len(script)", func(t *testing.T) {
bad := make([]byte, 64) // 64 is the word-size of a bitset.
bad[0] = byte(opcode.JMP)
bad[1] = 64
require.NoError(t, IsScriptCorrect(bad, nil))
})
t.Run("bad JMPL offset", func(t *testing.T) { t.Run("bad JMPL offset", func(t *testing.T) {
bad := getScript() bad := getScript()
bad[jmplOff+1] = 0xfe // into JMP bad[jmplOff+1] = 0xfe // into JMP
@ -212,10 +219,25 @@ func TestIsScriptCorrect(t *testing.T) {
t.Run("out of bounds TRY 2", func(t *testing.T) { t.Run("out of bounds TRY 2", func(t *testing.T) {
bad := getScript() bad := getScript()
bad[tryOff+2] = byte(len(bad) - tryOff) bad[tryOff+1] = byte(len(bad)-tryOff) + 1
require.Error(t, IsScriptCorrect(bad, nil)) require.Error(t, IsScriptCorrect(bad, nil))
}) })
t.Run("out of bounds TRY 2", func(t *testing.T) {
bad := getScript()
bad[tryOff+2] = byte(len(bad)-tryOff) + 1
require.Error(t, IsScriptCorrect(bad, nil))
})
t.Run("TRY with jumps to a len(script)", func(t *testing.T) {
bad := make([]byte, 64) // 64 is the word-size of a bitset.
bad[0] = byte(opcode.TRY)
bad[1] = 64
bad[2] = 64
bad[3] = byte(opcode.RET) // pad so that remaining script (PUSHINT8 0) is even in length.
require.NoError(t, IsScriptCorrect(bad, nil))
})
t.Run("bad TRYL offset 1", func(t *testing.T) { t.Run("bad TRYL offset 1", func(t *testing.T) {
bad := getScript() bad := getScript()
bad[trylOff+1] = byte(-(trylOff - jmpOff) - 1) // into "something" bad[trylOff+1] = byte(-(trylOff - jmpOff) - 1) // into "something"