abner / flutter_js

A Javascript engine to use with flutter. It uses quickjs on Android and JavascriptCore on IOS
MIT License
474 stars 123 forks source link

fetch does not escape strings with special characters #135

Open LilyStilson opened 1 year ago

LilyStilson commented 1 year ago

Bug

The title.

Current behavior

If you are trying to fetch from a remote API some JSON that will contain special characters (including \r\n\t, etc.) the response.json() will return you a string, not an object.

I made a repo and a Cloudflare worker for easier reproduction: LilyStilson/flutterjs_fetch_bug

Expected behavior

fetch properly escapes strings with special characters, parsing does not fail, response.json() is not returning a string, but an object and throws an exception (not fallbacks to response.text()) if it fails to parse JSON.

Remarks

I am not sure whether this is flutter_js issue or QuickJS issue. If it's the latter - I will transfer this issue to them. Also, their library (judging by their website) claims to support fetch and even some additional functions for JSON parsing which are not present in flutter_js.

LilyStilson commented 1 year ago

Diving a little bit deeper, looks like the problem is not with the fetch polyfill, but with the XMLHttpRequest implementation. I rewrote the callFetch() function in example like so:

async function callFetch(type) {
    function wrapper(url) {
        return new Promise((resolve, reject) => {
            const req = new XMLHttpRequest();
            req.open('GET', url, true);
            req.onload = () => req.status === 200 ? resolve(req.response) : reject(req.statusText);
            req.send();
        })
    }
    const response = await wrapper("https://flutterjs-test.nightskystudio.workers.dev/" + type);
    let json
    try {
        json = JSON.parse(response);
    } catch (e) {
        json = response;
    }
    return JSON.stringify({
      "result": json,
      "type": typeof(json)
    });
}

Something like a very-very simple wrapper around XMLHttpRequest, and yes, indeed, the problem is with its implementation.

LilyStilson commented 1 year ago

Looks like I'm talking to myself, but that's okay.

After I went through your code, I think that the problem is with the way you pass values around in XMLHttpRequest code. Somewhere there the original value returned from the http.get is getting lost or re-encoded so that it's impossible to parse later. The workaround I found is to use sendMessage() to call Dart and use its http package without any additional code on top of it. Now, the runtime abstraction looks like this:

class JSRuntime {
  final JavascriptRuntime _runtime = getJavascriptRuntime();

  static const _internalCode = """async function callFetch(type) {
    let message = await sendMessage("get", JSON.stringify({"url": "https://flutterjs-test.nightskystudio.workers.dev/" + type}))

    return JSON.stringify(message)
}""";

  JSRuntime() {
    _runtime.evaluate(_internalCode);

    _runtime.onMessage("get", (args) async {
      Uri url = Uri.parse(args["url"]);
      final response = await http.get(url);
      final json = jsonDecode(response.body);
      return json;
    });
  }

   // runtimeEval, runtimeEvalAsync, callFetch

}

Same way we can rewrite fetch function to be basically a call to sendMessage and do all the work in Dart.

LilyStilson commented 1 year ago

So... I purpose replacing fetch polyfill with this implementation

Javascript

function fetch(url, options) {
    options = options || {};
    return new Promise(async (resolve, reject) => {
        let request = await sendMessage("fetch", JSON.stringify({"url": url, "options": options}))

        const response = () => ({
            ok: ((request.status / 100) | 0) == 2, // 200-299
            statusText: request.statusText,
            status: request.status,
            url: request.responseURL,
            text: () => Promise.resolve(request.responseText),
            json: () => Promise.resolve(request.responseText).then(JSON.parse),
            blob: () => Promise.resolve(new Blob([request.response])),
            clone: response,
            headers: request.headers,
        })

        if (request.ok) resolve(response());
        else reject(response());
    });
}

Dart

_runtime.onMessage("fetch", (args) async {
  Uri url = Uri.parse(args["url"]);
  Map options = args["options"];
  Map<String, String> headers = (options["headers"] as Map<dynamic, dynamic>).map((key, value) => MapEntry("$key", "$value"));
  http.Response response;

  switch(options["method"]) {
    case "GET":
      response = await http.get(url, headers: headers);
      break;
    case "POST":
      response = await http.post(url, headers: headers, body: options["body"]);
      break;
    case "PUT":
      response = await http.put(url, headers: headers, body: options["body"]);
      break;
    case "DELETE":
      response = await http.delete(url, headers: headers);
      break;
    default:
      throw Exception("Invalid method");
  }

  final json = {
    "ok": response.statusCode >= 200 && response.statusCode < 300,
    "status": response.statusCode,
    "statusText": response.reasonPhrase,
    "headers": jsonEncode(response.headers),
    "body": response.body,
    "responseURL": response.request!.url.toString(),
    "responseText": response.body,
  };

  return json;
});

Of course, any improvements to JS/Dart implementation are very welcome. This fixes loss of special characters in response. I won't be creating a pull request for now, since I currently don't have time to find a way to incorporate this code into existing implementation, but everyone who stumbled across the same problem as I did can use this code to fix this.

cuifengcn commented 11 months ago

Thanks for the greate work! Solved \n \" .etc escape problems.

Gieted commented 7 months ago

The reason to escape character is lost is this line of code, which doesn't escape \ ('\"' in js is just ").

BTW literally nothing is escaped, so the owner of the page we're making requests to could easily perform an RCE attack.