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.

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

Configuration for disabling aliases #1093

Closed
d-lozanov opened this issue Dec 2, 2024 · 2 comments
Closed

Configuration for disabling aliases #1093

d-lozanov opened this issue Dec 2, 2024 · 2 comments

Comments

@d-lozanov
Copy link

d-lozanov commented Dec 2, 2024

Hi team,
As per this article, alias overloading could be used for DoS attacks. There are a few options for preventing these attacks like limiting the request body, limiting the number of aliases and disabling them. I couldn't find any configuration for disabling aliases and I had to implement a filter in my application.
Do you plan to provide some kind of configuration for disabling aliases( for example, application property ), so that people could use it out of the box, as it is a general problem?

I paste the solution with filter for a reference:

@WebFilter("/graphql")
public class AliasRejectionFilter extends OncePerRequestFilter {

    private final ObjectMapper objectMapper = new ObjectMapper();

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
            throws ServletException, IOException {
        if (!(request instanceof CachedBodyHttpServletRequest)) {
            request = new CachedBodyHttpServletRequest(request);
        }

        String body = ((CachedBodyHttpServletRequest) request).getCachedBody();
        String query = extractQueryFromBody(body);

        if (query == null || query.isEmpty()) {
            response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
            response.getWriter()
                    .write("{\"error\": \"Invalid GraphQL query.\"}");
            return;
        }

        if (containsAliases(query)) {
            response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
            response.getWriter()
                    .write("{\"error\": \"Aliases are not allowed in this query.\"}");
            return;
        }

        filterChain.doFilter(request, response);
    }

    private String extractQueryFromBody(String body) {
        try {
            JsonNode jsonNode = objectMapper.readTree(body);
            return jsonNode.get("query")
                    .asText();
        } catch (Exception e) {
            System.err.println("Failed to extract query from body: " + e.getMessage());
            return null;
        }
    }

    private boolean containsAliases(String query) {
        try {
            Document document = Parser.parse(query);

            List<OperationDefinition> operationDefinitions = document.getDefinitionsOfType(OperationDefinition.class);
            for (OperationDefinition opDef : operationDefinitions) {
                List<Selection> selections = opDef.getSelectionSet()
                        .getSelections();
                for (Selection selection : selections) {
                    if (selection instanceof Field) {
                        Field field = (Field) selection;
                        if (field.getAlias() != null) {
                            return true;
                        }
                    }
                }
            }
        } catch (Exception e) {
            return false;
        }
        return false;
    }

}

where CachedBodyHttpServletRequest is just a wrapper for the request.

Best regards,
Denis.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 2, 2024
@rstoyanchev
Copy link
Contributor

GraphQL Java has some configurable limits related to DoS. It would make sense for checks like these to be performed at that level too as they require understanding and parsing the query.

Please, consider asking in https://github.com/graphql-java/graphql-java.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Feb 13, 2025
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Feb 20, 2025
@rstoyanchev rstoyanchev reopened this Feb 20, 2025
@rstoyanchev rstoyanchev reopened this Feb 20, 2025
@rstoyanchev rstoyanchev closed this as not planned Won't fix, can't repro, duplicate, stale Feb 20, 2025
@rstoyanchev rstoyanchev removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants