Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Prevent Access to Tables not Specified in ABI #331

Closed
1 of 3 tasks
bytemaster opened this issue Sep 4, 2017 · 9 comments
Closed
1 of 3 tasks

Prevent Access to Tables not Specified in ABI #331

bytemaster opened this issue Sep 4, 2017 · 9 comments
Assignees
Milestone

Comments

@bytemaster
Copy link
Contributor

bytemaster commented Sep 4, 2017

  • verify the table type matches
  • verify that a table name can only be used for a single index
  • verify that the ABI definition doesn't define multiple tables with the same name
@bytemaster bytemaster added this to the EOS Dawn 1.0 milestone Sep 4, 2017
@bytemaster
Copy link
Contributor Author

These checks can be skipped during replay

@brianjohnson5972
Copy link
Contributor

Investigating.

@brianjohnson5972
Copy link
Contributor

  • verify that the ABI definition doesn't define multiple tables with the same name
    Already done by setcode call to AbiSerializer ctor, which calls setabi, which compares size of AbiSerializer map of tables to Abi vector of tables.

@brianjohnson5972
Copy link
Contributor

@bytemaster
"verify the table type matches"
There are 2 ways I can see this being done. It could be checked in the store/read_*** wasm_interface intrinsic methods, but this would be a lot of wasted checks, since each wasm store/read only needs to be verified once.
So I am thinking that this needs to be done in the load, and the ModuleInstance analyzed.

"verify that a table name can only be used for a single index"
This really should be "verify that every code's table name can only be used for a single index", right? And then doesn't the fact that we verify that the ABI definition doesn't define multiple tables with the same name and that we "verify that the table type matches" already ensure this?

Also, I am presuming these are only performed if an ABI is provided, are there any checks we want to perform if there is no ABI? Like ensure that table reads and writes are consistent with each other?

@brianjohnson5972
Copy link
Contributor

Would think that this information has to be available in the processed WASM structure, but having a hard time identifying it. If it isn't we could identify it as we process the WASM.

@brianjohnson5972
Copy link
Contributor

@bytemaster
Needs Clarification:

I am presuming the intent of this is safety (ensuring table access is consistent with ABI), not security, is that correct?

The statement about "these checks can be skipped during replay" sounds like these checks were planned to occur every time the code runs, but this seems to be a cost that shouldn't have to be incurred every time the code runs, vs. either using data in the existing loaded WASM structure (if it is possible) or else added to the WASM structure processing. If I don't get clarification on this I will plan to do the checks as the code is run, since that comment seems to assume this.

If the ABI is not provided, should a "lazy" ABI table definition be constructed as the code is run through, to ensure a table only has one type? Until I hear otherwise, I will assume the answer to this question is "yes".

For "verify that a table name can only be used for a single index", I will not do anything on this till I get clarification on exactly what it means.

@brianjohnson5972
Copy link
Contributor

"verify the table type matches"
Assuming this means that the indextype matches, since that is available in the code, but actually identifying the type involved would require parsing the wast.

Working on tests.

@brianjohnson5972
Copy link
Contributor

Still working on adding tests.

@heifner
Copy link
Contributor

heifner commented Nov 6, 2017

Resolved by #573

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants