Update TP: PHP - 83_array_variable_key

[felix-20]

Testability pattern

83_array_variable_key

Problem statement

The php code for instance 2 and instance 3 is the same.

 <?php
$a = "ttt";
$b = $_GET["p1"];
$c = $_GET["p2"];
$x = array(1,2,$a=>$b);
echo $x[$c];

Proposed changes

One possiblity for changing the code of instance 3 I came up with, was this:

<?php
$b = $_GET["p1"];
$c = $_GET["p2"]; // source
$x = array(1,2,$b=>$c);
echo $x[$b]; // sink

The key and the value in x are now user controlled. This is of course only one possibility.

Other

Do you have any other ideas? Or is it more useful to delete that instance?

[compaluca]

@felix-20, @enferas : on the same line of reasoning of @felix-20, cannot we go over the different combinations?

• key: variable; value: constant
• key: variable; value: variable
• key: constant; value: variable
• key: constant; value: constant For the key part, I guess we can enumerate the keys in the array and echo them?

[enferas]

In the original pattern there were comments on the two patterns https://github.com/enferas/TestabilityTarpits/blob/main/PHP/TestabilityPatterns/83_array_variable_key/Pattern%20Array%20Variable%20Key.md

They are the same but we are showing that the index of the array can point one time for the tainted element in the array and one time for a safe element. Thus, tools need to apply over or under approximation for this pattern.

[compaluca]

@enferas : some comments for you

Vulnerable or not vulnerable

• IMHO the reasoning here should be different. A pattern instance is either vulnerable or not regardeless of the inputs (your comments try to reflect the input a user can provide)
• basically if there is at least one possible input that triggers the vulnerability, then the instance is vulnerable
• given this I would suggest to keep only one of the duplicated instances

inconsistency in the original repo: the original README file in https://github.com/enferas/TestabilityTarpits/blob/main/PHP/TestabilityPatterns/83_array_variable_key/Pattern%20Array%20Variable%20Key.md is not in sync with the instances in that patterns. E.g., there are three instances in the folder and 4 in the README

combinations: what about trying to test also cases in which the attacker controlled part goes in the key? see my previous comment (here)[https://github.com/testable-eu/sast-testability-patterns/issues/39#issuecomment-1573962025]