Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BaseURL config vs absolute URL #5902

Open
emyei opened this issue Sep 18, 2023 · 3 comments · May be fixed by #6192
Open

BaseURL config vs absolute URL #5902

emyei opened this issue Sep 18, 2023 · 3 comments · May be fixed by #6192

Comments

@emyei
Copy link

emyei commented Sep 18, 2023 •
edited
Loading

Is your feature request related to a problem? Please describe.

I think that less experienced developers are unaware of how Axios behaves in terms of prioritizing absolute URLs over the BaseURL attribute configuration.

This situation could generate SSRF type vulnerabilities in use cases such as:

  config.defaults.BaseURL = 'http://mysite.com';
  path = req.query.path;
  axios.get(path);

A malicious user could enter path values ​​like 'http://evil.com/' (absolute URL) and direct the request to a different site.

Describe the solution you'd like

While it is possible to validate that user input does not contain absolute URLs, I find it safer to have a configuration attribute that allows or denies absolute URLs. Example:

allowAbsoluteURL = false/true

The value true would reflect normal Axios behavior.

The value false generates an exception in requests when each of the following conditions are met:

  • The restclient methods receives an absolute URL as a parameter.
  • The value of BaseURL config was explicitly declared in the restclient configuration.
  • The BaseURL value is not a prefix to the absolute URL received as a parameter.

Describe alternatives you've considered

No response

Additional context/Screenshots

No response
@fmunozs
Copy link

fmunozs commented Sep 18, 2023

Hello team, IMHO this could be a security issue, there is no mention on the documentation as far as I could see, that a full URL will always be parsed as such after setting the BaseURL attribute.

I know that it could be a breaking change, but by default if BaseURL is set it should never try to identify if the path parameter is a full and always attempt to create the final URL using the baseURL+path.

@Traveller08
Copy link

Hi
I would like to work on this issue. Please assign it to me.

@GethosTheWalrus GethosTheWalrus linked a pull request Jan 18, 2024 that will close this issue
Open
GethosTheWalrus added a commit to GethosTheWalrus/axios that referenced this issue Jan 18, 2024
@GethosTheWalrus
feat: update tests supporting issue axios#5902 functionality
e9ac9ec
@GethosTheWalrus
Copy link

GethosTheWalrus commented Jan 18, 2024 •
edited
Loading

Just pushed a PR allowing you to change axios.defaults.allowAbsoluteUrls or supply a config param called allowAbsoluteUrls with a request to modify this behavior. Setting the param to false will cause the specified path to be combined with the base URL regardless of whether or not it is absolute. This value is true by default, preserving the original behavior.

Minimal example: Will make a request to https://www.miketoscano.com/https://www.google.com (combined urls)

import axios from './axios.js';

axios.defaults.baseURL = 'https://www.miketoscano.com';
axios.defaults.allowAbsoluteUrls = false;

console.log(axios.defaults);

axios.get('https://www.google.com').then( (response) => {
    console.log(response);
    console.log('done');
});

Minimal example 2: Will make a request to http://someurl.com/http://someotherurl.com/ (combined urls)

const instance = axios.create({
      baseURL: 'http://someurl.com/',
      allowAbsoluteUrls: false
    });

    instance.get('http://someotherurl.com/');
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Add config for ignoring absolute URLs (#5902) GethosTheWalrus/axios
6 participants
@fmunozs @jasonsaayman @GethosTheWalrus @emyei @Traveller08 @DavidJDallas