Skip to content

Commit

Permalink
fix: prevent sha256() from throwing on statically known strings and…
Browse files Browse the repository at this point in the history
… document potential issues with runtime hashing (#907)
  • Loading branch information
novusnota authored Nov 14, 2024
1 parent 5f4fae6 commit 387238c
Show file tree
Hide file tree
Showing 14 changed files with 83 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Docs: compilation of examples in `data-structures.mdx` and across Cookbook: PR [#917](https://github.com/tact-lang/tact/pull/917)
- `as coins` map value serialization type is now handled correctly: PR [#987](https://github.com/tact-lang/tact/pull/987)
- Type checking for `foreach` loops in trait methods: PR [#1017](https://github.com/tact-lang/tact/pull/1017)
- The `sha256()` function no longer throws on statically known strings of any length: PR [#907](https://github.com/tact-lang/tact/pull/907)

### Release contributors

Expand Down
2 changes: 1 addition & 1 deletion docs/links-to-web-ide.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export default function remarkLinksToWebIDE() {
// Constructing opening <a> tag
[
// Open the tag
'<a',
'<a data-pagefind-ignore="all"',
// Make links opened in new tab
'target="_blank"',
// Set styles
Expand Down
4 changes: 2 additions & 2 deletions docs/src/content/docs/book/operators.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ Unary here means that they are applied only to one operand of the given expressi

Unary operators can be one of the two types:

* prefix — placed before the expression.
* postfix (or suffix) — placed after the expression.
* Prefix — placed before the expression.
* Postfix (or suffix) — placed after the expression.

### Non-null assert, `!!` {#unary-non-null-assert}

Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/book/statements.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ The destructuring assignment is a concise way to unpack [Structs][s] and [Messag

The syntax is derived from the [`let` statement](#let), and instead of specifying the variable name directly it involves specifying the structure type on the left side of the [assignment operator `={:tact}`](/book/operators#assignment), which corresponds to the structure type of the value on the right side.

```tact {6}
```tact {9}
// Definition of Example
struct Example { number: Int }
Expand Down
13 changes: 10 additions & 3 deletions docs/src/content/docs/ref/core-math.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,17 @@ fun sha256(data: Slice): Int;
fun sha256(data: String): Int;
```

Computes and returns the [SHA-256][sha-2] hash as an $256$-bit unsigned [`Int{:tact}`][int] from a passed [`Slice{:tact}`][slice] or [`String{:tact}`][p] `data`.
Computes and returns the [SHA-256][sha-2] hash as a $256$-bit unsigned [`Int{:tact}`][int] from a passed [`Slice{:tact}`][slice] or [`String{:tact}`][p] `data`.

In case `data` is a [`String{:tact}`][p] it should have a number of bits divisible by $8$, and in case it's a [`Slice{:tact}`][slice] it must **also** have no references (i.e. only up to 1023 bits of data in total).
In case `data` is a [`String{:tact}`][p] it should have a number of bits divisible by $8$, and in case it's a [`Slice{:tact}`][slice] it must **also** have no references (i.e. only up to $1023$ bits of data in total). This function tries to resolve constant string values at [compile-time](/ref/core-comptime) whenever possible.

This function tries to resolve constant string values in [compile-time](/ref/core-comptime) whenever possible.
:::caution

If the [`String{:tact}`][p] value cannot be resolved during [compilation time](/ref/core-comptime), then the hash is calculated at runtime by the [TVM][tvm] itself. Note, that hashing strings with more than $128$ bytes by the [TVM][tvm] can cause collisions if their first $128$ bytes are the same.

Therefore, prefer using statically known strings whenever possible. When in doubt, use strings of up to $128$ bytes long.

:::

Usage examples:

Expand All @@ -277,5 +283,6 @@ sha256(someVariableElsewhere); // will try to resolve at compile-time,
[int]: /book/integers
[slice]: /book/cells#slices

[tvm]: https://docs.ton.org/learn/tvm-instructions/tvm-overview
[ed]: https://en.wikipedia.org/wiki/EdDSA#Ed25519
[sha-2]: https://en.wikipedia.org/wiki/SHA-2#Hash_standard
2 changes: 1 addition & 1 deletion docs/src/content/docs/ref/stdlib-dns.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Source code (FunC): [dns.fc#L1](https://github.com/tact-lang/tact/blob/e69c7fc99
native dnsInternalNormalize(src: Slice): Slice;
```

Normalizes the internal DNS representation of the [`Slice{:tact}`][slice]. The passed [`Slice{:tact}`][slice] must not have any references, otherwise an exception wit [exit code 134](/book/exit-codes#134) will be thrown: `Invalid argument`.
Normalizes the internal DNS representation of the [`Slice{:tact}`][slice]. The passed [`Slice{:tact}`][slice] must not have any references, otherwise an exception with [exit code 134](/book/exit-codes#134) will be thrown: `Invalid argument`.

Source code (FunC): [dns.fc#L125](https://github.com/tact-lang/tact/blob/e69c7fc99dc9be3fa5ff984456c03ffe8fed3677/stdlib/libs/dns.fc#L125)

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"release": "yarn clean && yarn build && yarn coverage && yarn release-it --npm.yarn1",
"lint": "yarn eslint .",
"lint:schema": "ajv validate -s schemas/configSchema.json -d tact.config.json",
"fmt": "yarn prettier -w .",
"fmt": "yarn prettier -l -w .",
"fmt:check": "yarn prettier --check .",
"spell": "yarn cspell --no-progress \"**\"",
"knip": "knip",
Expand Down
27 changes: 17 additions & 10 deletions src/abi/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
writeExpression,
writeValue,
} from "../generator/writers/writeExpression";
import { throwCompilationError } from "../errors";
import { TactConstEvalError, throwCompilationError } from "../errors";
import { evalConstantExpression } from "../constEval";
import { getErrorId } from "../types/resolveErrors";
import { AbiFunction } from "./AbiFunction";
Expand Down Expand Up @@ -368,23 +368,30 @@ export const GlobalFunctions: Map<string, AbiFunction> = new Map([

// String case
if (arg0.name === "String") {
let str: string | undefined;

// Try const-eval
try {
const str = evalConstantExpression(
str = evalConstantExpression(
resolved[0]!,
ctx.ctx,
) as string;
if (Buffer.from(str).length > 128) {
throwCompilationError(
"sha256 expects string argument with byte length <= 128",
ref,
);
}
} catch (error) {
if (
!(error instanceof TactConstEvalError) ||
error.fatal
)
throw error;
}

// If const-eval did succeed
if (str !== undefined) {
return BigInt(
"0x" + sha256_sync(str).toString("hex"),
).toString(10);
} catch (e) {
// Not a constant
}

// Otherwise, revert back to runtime hash through SHA256U
const exp = writeExpression(resolved[0]!, ctx);
return `string_hash(${exp})`;
}
Expand Down
15 changes: 6 additions & 9 deletions src/interpreter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Address, beginCell, BitString, Cell, toNano } from "@ton/core";
import { Address, beginCell, BitString, Cell, Slice, toNano } from "@ton/core";
import { paddedBufferToBits } from "@ton/core/dist/boc/utils/paddedBits";
import * as crc32 from "crc-32";
import { evalConstantExpression } from "./constEval";
Expand Down Expand Up @@ -1068,17 +1068,14 @@ export class Interpreter {
}
case "sha256": {
ensureFunArity(1, ast.args, ast.loc);
const str = ensureString(
this.interpretExpression(ast.args[0]!),
ast.args[0]!.loc,
);
const dataSize = Buffer.from(str).length;
if (dataSize > 128) {
throwErrorConstEval(
`data is too large for sha256 hash, expected up to 128 bytes, got ${dataSize}`,
const expr = this.interpretExpression(ast.args[0]!);
if (expr instanceof Slice) {
throwNonFatalErrorConstEval(
"slice argument is currently not supported",
ast.loc,
);
}
const str = ensureString(expr, ast.args[0]!.loc);
return BigInt("0x" + sha256_sync(str).toString("hex"));
}
case "emptyMap": {
Expand Down
13 changes: 13 additions & 0 deletions src/test/compilation-failed/abi-global-errors.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { __DANGER_resetNodeId } from "../../grammar/ast";
import { itShouldNotCompile } from "./util";

describe("abi/global.ts errors", () => {
beforeEach(() => {
__DANGER_resetNodeId();
});

itShouldNotCompile({
testName: "sha256-expects-string-or-slice",
errorMessage: "sha256 expects string or slice argument",
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
contract Sha256 {
val: Int = 0;
receive() {
sha256(self.val);
}
}
5 changes: 5 additions & 0 deletions src/test/compilation-failed/tact.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@
"name": "scope-const-shadows-stdlib-ident",
"path": "./contracts/scope-const-shadows-stdlib-ident.tact",
"output": "./contracts/output"
},
{
"name": "sha256-expects-string-or-slice",
"path": "./contracts/sha256-expects-string-or-slice.tact",
"output": "./contracts/output"
}
]
}
10 changes: 9 additions & 1 deletion src/test/e2e-emulated/contracts/intrinsics.tact
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ contract IntrinsicsTester {
return sha256(src);
}

get fun getHashLongComptime(): Int {
return sha256("------------------------------------------------------------------------------------------------------------------------------129");
}

get fun getHashLongRuntime(src: String): Int {
return sha256(src);
}

receive("emit_1") {
emit("Hello world".asComment());
}
Expand Down Expand Up @@ -231,4 +239,4 @@ contract IntrinsicsTester {
get fun getRawSlice24(): Slice {
return self.v;
}
}
}
10 changes: 10 additions & 0 deletions src/test/e2e-emulated/intrinsics.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,16 @@ describe("intrinsics", () => {
),
).toBe(sha256("sometest"));
expect(await contract.getGetHash4("wallet")).toBe(sha256("wallet"));
const longString =
"------------------------------------------------------------------------------------------------------------------------------129";
expect(await contract.getGetHashLongComptime()).toBe(
sha256(longString),
);
// NOTE: The discrepancy here is expected, since SHA256U operates only on the first 127 bytes
expect(
(await contract.getGetHashLongRuntime(longString)) !==
sha256(longString),
).toBe(true);

// Check `slice`
expect(
Expand Down

0 comments on commit 387238c

Please sign in to comment.