From d9b2699843e237ebfa4303d8a744670a827083ea Mon Sep 17 00:00:00 2001 From: Hang Su <87964331+ahangsu@users.noreply.github.com> Date: Fri, 26 Aug 2022 11:53:02 -0400 Subject: [PATCH] Enhancement: Deprecating use of langspec (#367) Co-authored-by: Michael Diamant --- .../algosdk/crypto/LogicsigSignature.java | 68 +++++++++++++++---- .../com/algorand/algosdk/logic/Logic.java | 9 ++- .../algosdk/crypto/TestLogicsigSignature.java | 10 --- .../algosdk/unit/ProgramSanityCheck.java | 37 ++++++++++ src/test/unit.tags | 1 + 5 files changed, 101 insertions(+), 24 deletions(-) create mode 100644 src/test/java/com/algorand/algosdk/unit/ProgramSanityCheck.java diff --git a/src/main/java/com/algorand/algosdk/crypto/LogicsigSignature.java b/src/main/java/com/algorand/algosdk/crypto/LogicsigSignature.java index 91c74de45..7a3b92ffc 100644 --- a/src/main/java/com/algorand/algosdk/crypto/LogicsigSignature.java +++ b/src/main/java/com/algorand/algosdk/crypto/LogicsigSignature.java @@ -16,6 +16,7 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import org.apache.commons.codec.binary.Base64; /** * Serializable logicsig class. @@ -39,6 +40,59 @@ public class LogicsigSignature { @JsonProperty("msig") public MultisigSignature msig; + + private static boolean isAsciiPrintable(final byte symbol) { + // linebreak existence check in program byte + boolean isBreakLine = symbol == '\n'; + // printable ascii between range 32 (space) and 126 (tilde ~) + boolean isStdPrintable = symbol >= ' ' && symbol <= '~'; + return isBreakLine || isStdPrintable; + } + + private static boolean isAsciiPrintable(final byte[] program) { + for (byte b : program) { + if (!isAsciiPrintable(b)) + return false; + } + return true; + } + + /** + * Performs heuristic program validation: + * check if passed in bytes are Algorand address or is B64 encoded, rather than Teal bytes + * @param program + */ + private static void sanityCheckProgram(final byte[] program) { + if (program == null || program.length == 0) + throw new IllegalArgumentException("empty program"); + // in any case, if a slice of "program-bytes" is full of ASCII printable, + // then the slice of bytes can't be Teal program bytes. + // need to check what possible kind of bytes are passed in. + if (isAsciiPrintable(program)) { + // maybe the bytes passed in are representing an Algorand address + boolean isAddress = false; + try { + new Address(new String(program)); + isAddress = true; + } catch (NoSuchAlgorithmException | IllegalArgumentException e) { + // if exception is IllegalArgException, it means bytes are not Algorand address + if (e instanceof NoSuchAlgorithmException) + throw new IllegalArgumentException("cannot check if program bytes are Algorand address" + e); + } + if (isAddress) + throw new IllegalArgumentException("requesting program bytes, get Algorand address"); + + // or maybe these bytes are some B64 encoded bytes representation + if (Base64.isBase64(program)) + throw new IllegalArgumentException("program should not be b64 encoded"); + + // can't further analyze, but it is more than just B64 encoding at this point + throw new IllegalArgumentException( + "program bytes are all ASCII printable characters, not looking like Teal byte code" + ); + } + } + @JsonCreator public LogicsigSignature( @JsonProperty("l") byte[] logic, @@ -48,14 +102,8 @@ public LogicsigSignature( ) { this.logic = Objects.requireNonNull(logic, "program must not be null"); this.args = args; - boolean verified = false; - try { - verified = Logic.checkProgram(this.logic, this.args); - } catch (IOException ex) { - throw new IllegalArgumentException("invalid program", ex); - } - assert verified; + sanityCheckProgram(this.logic); if (sig != null) this.sig = new Signature(sig); this.msig = msig; @@ -124,11 +172,7 @@ public boolean verify(Address singleSigner) throws NoSuchAlgorithmException { return false; } - try { - Logic.checkProgram(this.logic, this.args); - } catch (Exception ex) { - return false; - } + sanityCheckProgram(this.logic); PublicKey pk; try { diff --git a/src/main/java/com/algorand/algosdk/logic/Logic.java b/src/main/java/com/algorand/algosdk/logic/Logic.java index ef4202e08..42965e295 100644 --- a/src/main/java/com/algorand/algosdk/logic/Logic.java +++ b/src/main/java/com/algorand/algosdk/logic/Logic.java @@ -14,7 +14,11 @@ /** * Logic class provides static checkProgram function * that can be used for client-side program validation for size and execution cost. + * + * @deprecated this class is deprecated for relying on metadata (`langspec.json`) that + * does not accurately represent opcode behavior across program versions. */ +@Deprecated public class Logic { private static final int MAX_COST = 20000; @@ -103,7 +107,7 @@ protected static class ByteConstBlock { * Each byte in a varint, except the last byte, has the most significant * bit (msb) set – this indicates that there are further bytes to come. * The lower 7 bits of each byte are used to store the two's complement - * representation of the number in groups of 7 bits, least significant + * representation of the number in groups of 7 bits, the least significant * group first. * https://developers.google.com/protocol-buffers/docs/encoding * @param value being serialized @@ -128,7 +132,7 @@ public static byte[] putUVarint(int value) { * Given a varint, get the integer value * @param buffer serialized varint * @param bufferOffset position in the buffer to start reading from - * @return pair of values in in array: value, read size + * @return pair of values in an array: value, read size */ public static VarintResult getUVarint(byte [] buffer, int bufferOffset) { int x = 0; @@ -164,6 +168,7 @@ public static boolean checkProgram(byte[] program, List args) throws IOE /** * Performs basic program validation: instruction count and program cost + * * @param program Program to validate * @param args Program arguments to validate * @return boolean diff --git a/src/test/java/com/algorand/algosdk/crypto/TestLogicsigSignature.java b/src/test/java/com/algorand/algosdk/crypto/TestLogicsigSignature.java index e31721bb3..673fbe82e 100644 --- a/src/test/java/com/algorand/algosdk/crypto/TestLogicsigSignature.java +++ b/src/test/java/com/algorand/algosdk/crypto/TestLogicsigSignature.java @@ -62,16 +62,6 @@ public void testLogicsigCreation() throws Exception { assertThat(lsig).isEqualTo(lsig1); } - @Test - public void testLogicsigInvalidProgramCreation() throws Exception { - byte[] program = { - 0x7F, 0x20, 0x01, 0x01, 0x22 - }; - assertThatThrownBy(() -> new LogicsigSignature(program)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("unsupported version"); - } - @Test public void testLogicsigSignature() throws Exception { byte[] program = { diff --git a/src/test/java/com/algorand/algosdk/unit/ProgramSanityCheck.java b/src/test/java/com/algorand/algosdk/unit/ProgramSanityCheck.java new file mode 100644 index 000000000..6e9ffde03 --- /dev/null +++ b/src/test/java/com/algorand/algosdk/unit/ProgramSanityCheck.java @@ -0,0 +1,37 @@ +package com.algorand.algosdk.unit; + +import com.algorand.algosdk.crypto.LogicsigSignature; +import com.algorand.algosdk.util.Encoder; + +import io.cucumber.java.en.Given; +import io.cucumber.java.en.Then; +import io.cucumber.java.en.When; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ProgramSanityCheck { + byte[] seeminglyProgram; + String actualErrMsg; + + @Given("a base64 encoded program bytes for heuristic sanity check {string}") + public void takeB64encodedBytes(String b64encodedBytes) { + seeminglyProgram = Encoder.decodeFromBase64(b64encodedBytes); + } + + @When("I start heuristic sanity check over the bytes") + public void heuristicCheckOverBytes() { + try { + new LogicsigSignature(seeminglyProgram); + } catch (Exception e) { + actualErrMsg = e.getMessage(); + } + } + + @Then("if the heuristic sanity check throws an error, the error contains {string}") + public void checkErrorIfMatching(String errMsg) { + if (errMsg != null && !errMsg.isEmpty()) + assertThat(actualErrMsg).contains(errMsg); + else + assertThat(actualErrMsg).isNullOrEmpty(); + } +} diff --git a/src/test/unit.tags b/src/test/unit.tags index bfad8a343..f51517453 100644 --- a/src/test/unit.tags +++ b/src/test/unit.tags @@ -12,6 +12,7 @@ @unit.indexer.logs @unit.indexer.rekey @unit.offline +@unit.program_sanity_check @unit.rekey @unit.responses @unit.responses.231