Skip to content

Commit ede2c39

Browse files
committed
Require explicit contract for --replace-call-with-contract
Change --replace-call-with-contract to produce a hard error instead of a warning when used with functions lacking explicit contracts. This change addresses a soundness issue where CBMC would previously assume a trivial contract automatically. Users that really need a trivial contract with no constraints should use ````c int my_function(int x) __CPROVER_requires(1) __CPROVER_ensures(1) { return x + 1; } ```` Fixes: #8728
1 parent 4fe3ade commit ede2c39

File tree

5 files changed

+56
-25
lines changed

5 files changed

+56
-25
lines changed
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#include <assert.h>
2+
3+
int foo(int x)
4+
{
5+
return x + 1;
6+
}
7+
8+
int main()
9+
{
10+
int result = foo(5);
11+
assert(result == 6);
12+
return 0;
13+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CORE
2+
main.c
3+
--replace-call-with-contract foo
4+
^Function 'foo' does not have a contract\. When using --replace-call-with-contract, a contract must be explicitly provided\. If you need a trivial contract, please add explicit __CPROVER_requires and __CPROVER_ensures clauses to the function\.$
5+
^EXIT=6$
6+
^SIGNAL=0$
7+
--
8+
--
9+
Checks that attempting to replace a function call without a contract produces
10+
an error instead of assuming a trivial contract. This addresses the soundness
11+
risk identified in issue https://github.com/diffblue/cbmc/issues/8728.

src/goto-instrument/contracts/contracts.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Date: February 2016
1414
#include "contracts.h"
1515

1616
#include <util/c_types.h>
17+
#include <util/exception_utils.h>
1718
#include <util/format_expr.h>
1819
#include <util/fresh_symbol.h>
1920
#include <util/mathematical_expr.h>
@@ -616,6 +617,22 @@ void code_contractst::apply_function_contract(
616617
// Isolate each component of the contract.
617618
const auto &type = get_contract(target_function, ns);
618619

620+
// Check if the function actually has a contract
621+
// If not, produce a hard error for soundness
622+
const symbolt *contract_sym;
623+
if(ns.lookup("contract::" + id2string(target_function), contract_sym))
624+
{
625+
// no contract provided in the source - this is a soundness issue
626+
// when using --replace-call-with-contract
627+
throw invalid_input_exceptiont(
628+
"Function '" + id2string(target_function) +
629+
"' does not have a contract. " +
630+
"When using --replace-call-with-contract, a contract must be "
631+
"explicitly " +
632+
"provided. If you need a trivial contract, please add explicit " +
633+
"__CPROVER_requires and __CPROVER_ensures clauses to the function.");
634+
}
635+
619636
// Prepare to instantiate expressions in the callee
620637
// with expressions from the call site (e.g. the return value).
621638
exprt::operandst instantiation_values;

src/goto-instrument/contracts/doc/user/contracts-cli.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ goto-instrument [--apply-loop-contracts] [--enforce-contract <function>] (--repl
1111
Where:
1212
- `--apply-loop-contracts` is optional and specifies to apply loop contracts globally;
1313
- `--enforce-contract <function>` is optional and specifies that `function` must be checked against its contract.
14-
- `--replace-call-with-contract <function>` is optional and specifies that all calls to `function` must be replaced with its contract;
14+
- `--replace-call-with-contract <function>` is optional and specifies that all calls to `function` must be replaced with its contract.
15+
It is an error if `function` does not have a contract.
1516

1617
## Applying the function contracts transformation (with the dynamic frames method)
1718

@@ -27,5 +28,4 @@ Where:
2728
When `contract` is not specfied, the contract is assumed to be carried by the `function` itself.
2829
- `--replace-call-with-contract <function>[/<contract>]` is optional and specifies that all calls to `function` must be replaced with `contract`.
2930
When `contract` is not specfied, the contract is assumed to be carried by the `function` itself.
30-
31-
31+
It is an error if `function` does not have a contract.

src/goto-instrument/contracts/dynamic-frames/dfcc_contract_handler.cpp

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ Date: August 2022
99

1010
#include "dfcc_contract_handler.h"
1111

12+
#include <util/exception_utils.h>
13+
1214
#include <goto-programs/goto_model.h>
1315
#include <goto-programs/remove_function_pointers.h>
1416

@@ -118,7 +120,8 @@ const symbolt &dfcc_contract_handlert::get_pure_contract_symbol(
118120
{
119121
// The contract symbol might not have been created if the function had
120122
// no contract or a contract with all empty clauses (which is equivalent).
121-
// in that case we create a fresh symbol again with empty clauses.
123+
// This is a soundness issue when using --replace-call-with-contract
124+
// because we should not assume a trivial contract.
122125
PRECONDITION_WITH_DIAGNOSTICS(
123126
function_id_opt.has_value(),
124127
"Contract '" + pure_contract_id +
@@ -129,27 +132,14 @@ const symbolt &dfcc_contract_handlert::get_pure_contract_symbol(
129132
const auto &function_symbol =
130133
dfcc_utilst::get_function_symbol(goto_model.symbol_table, function_id);
131134

132-
log.warning() << "Contract '" << contract_id
133-
<< "' not found, deriving empty pure contract '"
134-
<< pure_contract_id << "' from function '" << function_id
135-
<< "'" << messaget::eom;
136-
137-
symbolt new_symbol{
138-
pure_contract_id, function_symbol.type, function_symbol.mode};
139-
new_symbol.base_name = pure_contract_id;
140-
new_symbol.pretty_name = pure_contract_id;
141-
new_symbol.is_property = true;
142-
new_symbol.module = function_symbol.module;
143-
new_symbol.location = function_symbol.location;
144-
auto entry = goto_model.symbol_table.insert(std::move(new_symbol));
145-
INVARIANT(
146-
entry.second,
147-
"contract '" + id2string(function_symbol.display_name()) +
148-
"' already set at " + id2string(entry.first.location.as_string()));
149-
// this lookup will work and set the pointer
150-
// no need to check for signature compatibility
151-
ns.lookup(pure_contract_id, pure_contract_symbol);
152-
return *pure_contract_symbol;
135+
// Produce a hard error instead of assuming a trivial contract
136+
// to address soundness risk
137+
throw invalid_input_exceptiont(
138+
"Function '" + id2string(function_id) + "' does not have a contract. " +
139+
"When using --replace-call-with-contract, a contract must be "
140+
"explicitly " +
141+
"provided. If you need a trivial contract, please add explicit " +
142+
"__CPROVER_requires and __CPROVER_ensures clauses to the function.");
153143
}
154144
}
155145

0 commit comments

Comments
 (0)