Skip to content

Commit

Permalink
Clean up deep_write implementation
Browse files Browse the repository at this point in the history
Summary:
This diff:
- constructs the full_path before performing a raw_read() to look up the memory location to update. This could avoid potential inconsistencies. Eg. a write to the same final path can be triggered through calls to deep_write() with different arguments: FieldMemoryLocation(root, .field) Or MemoryLocation(root)+ Path(.field) arguments.
- Makes a copy of the taint before applying aliasing properties.
- Updates the log messages.

Reviewed By: yuhshin-oss

Differential Revision: D68126282

fbshipit-source-id: 5955b50cf48569fac90e402c834d056cb56b3f5a
  • Loading branch information
Anwesh Tuladhar authored and facebook-github-bot committed Jan 14, 2025
1 parent f810b76 commit 3957161
Showing 1 changed file with 19 additions and 16 deletions.
35 changes: 19 additions & 16 deletions source/TaintEnvironment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,23 @@ void TaintEnvironment::deep_write(
const Path& path,
TaintTree taint,
UpdateKind kind) {
const auto& points_to_tree = resolved_aliases.get(memory_location->root());

auto full_path = memory_location->path();
full_path.extend(path);

LOG(5,
"{} update taint tree at: {} path `{}` with {}",
kind == UpdateKind::Strong ? "Strong" : "Weak",
show(memory_location),
path,
show(memory_location->root()),
full_path,
taint);

const auto& points_to_tree = resolved_aliases.get(memory_location->root());

auto [remaining_path, target_memory_locations] =
points_to_tree.raw_read_max_path(memory_location->path());

Path full_path = remaining_path;
full_path.extend(path);
// Manually destructuring a pair as cpp17 doesn't allow us to capture
// destructured variables in lambda below.
auto raw_read_result = points_to_tree.raw_read_max_path(full_path);
auto remaining_path = raw_read_result.first;
auto target_memory_locations = raw_read_result.second;

if (kind == UpdateKind::Strong && target_memory_locations.root().size() > 1) {
// In practice, only one of the memory location is affected, so we must
Expand All @@ -148,21 +151,21 @@ void TaintEnvironment::deep_write(

for (const auto& [target_memory_location, properties] :
target_memory_locations.root()) {
taint.apply_aliasing_properties(properties);
auto taint_to_write = taint;
taint_to_write.apply_aliasing_properties(properties);

LOG(5,
"{} updating taint tree of {}. Root: {}, {} with: {}",
"{} updating taint tree of {} at {} with: {}",
kind == UpdateKind::Strong ? "Strong" : "Weak",
show(target_memory_location),
show(target_memory_location->root()),
full_path,
taint);
remaining_path,
taint_to_write);

environment_.update(
target_memory_location,
[&full_path, &taint, kind](const TaintTree& tree) {
[&remaining_path, &taint_to_write, kind](const TaintTree& tree) {
auto copy = tree;
copy.write(full_path, taint, kind);
copy.write(remaining_path, std::move(taint_to_write), kind);
return copy;
});
}
Expand Down

0 comments on commit 3957161

Please sign in to comment.