Question about DNS Rebinding unit test

[craig-davis]

I have a concern that the DNS rebinding test may be ambiguous in asserting the correct response.

In the rebinding test it appears that the code sets check = 1 in the initial success .then on line 76, and in the .catch if the status code is a 400 on line 80.

ssrf-req-filter/test/index.js

Lines 73 to 88 in 170b64d

	const response = await axios.get(url, {httpAgent: ssrfFilter(url),
	httpsAgent: ssrfFilter(url)})
	.then((response) => {
	check = 1;
	})
	.catch((error) => {
	if (error.message === 'Request failed with status code 400') {
	check = 1;
	} else {
	check = 0;
	}
	})
	.then(() => {
	return check;
	});
	expect(response).to.equal(1);

The test then asserts that check has a value of 1. This seems to say that the test will pass with either a 200 or a 400 response from the server.

Is this the correct interpretation of this test?

[y-mehta]

@craig-davis, I agree; it's ambiguous and need to figure out a better approach to handle it.

The rebind.it URL being used for the test resolves to a public IP first and subsequently resolves to an internal IP (127.0.0.1) - If response from the public server is received, it'll pass and fail otherwise. So, the test was added to ensure it's safe from DSN rebinding.

[craig-davis]

What about using sinon to spy on the checkIp() function?

The function would have to get relocated so it could be accessed. Having a spy on it or a stub in place would allow the tests to make assertions about the IP it was called with and how many times it's been called.