Include offset for cropped fields

[curquiza]

Originally posted here by @solonovamax

I spent like 20 minutes writing an issue and then my browser tab decided to crash and killed all my progress, and I'm kinda annoyed about that so I'm just gonna be brief here and add more details later.

Currently, when both showMatchesPosition and attributesToCrop are in use, it is difficult to use the matches position as you do not know where the crop begins.

I would like to propose adding [field]_start and [field]_length properties to the returned json when searching. For example, searching for minim might return this result:

{
  "id": 1,
  "title": "Lorem Ipsum",
  "content": "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.",
  "_formatted": {
    "id": 1,
    "title": "Lorem Ipsum",
    "content": "…Ut enim ad minim veniam…",
    "content_start": 124,
    "content_length": 25, // note: the length here includes the added crop marker ('…')
  },
  "_matchesPosition": {
    "content": [
      {
        "start": 135,
        "length": 5
      }
    ]
  }
}

then, this can be used as follows (in pseudocode)

result = /* ... */

val content = result["_formatted"]["content"]
val contentOffset = result["_formatted"]["content_start"]

val match = result["_matchesPosition"]["content"][0]
val matchOffset = match["start"] - contentOffset
val matchLength = match["length"]

// use the computed match and the content for highlighting

my specific usecase: I want to add match highlighting to search results for my application. However, I cannot use the built-in highlighting as that would wrap html tags around the matches, as the content is user-supplied. So if I didn't escape the html, it would open up an avenue to XSS. But since it's escaped, wrapping the matches with html tags does not work.
And, I can't use the offset provided in the match positions, as I do not know where the content was cropped, so I don't know where the match is.

An alternative syntax to the one I suggested could be:

{
  "id": 1,
  "title": "Lorem Ipsum",
  "content": "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.",
  "_formatted": {
    "id": 1,
    "title": "Lorem Ipsum",
    "content": "…Ut enim ad minim veniam…"
  },
  "_crops": {
    "content": { // the name of the field the content was cropped from
      "start": 124,
      "length": 25
    }
  }
  "_matchesPosition": {
    "content": [
      {
        "start": 135,
        "length": 5
      }
    ]
  }
}

[curquiza]

@solonovamax feel free to add more details about your needs here

[MatthiasvB]

Yes, this would be an important feature for me as well. I will not use any HTML prerendering feature to show highlights due to XSS concerns, but I do want to use the cropping feature for search results and I do want to highlight matches. This is currently plainly impossible because, while I get a cropped string, I only have information about the matches' positions in the uncropped attribute. Effectively, this means there is no way to securely highlight matches in cropped attributes. Would it be going too far to call this a potential security issue?

To avoid further processing of the response, it'd be ideal to get match positions within the cropped attribute to avoid having to manually calculate offsets.

And while we're at it, it'd be much more usable to (at least optionally) get these indexes as char not byte indices.

Or, even better, actually get the entire response as segments of text that are marked as highlighted or not highlighted.

Below Typescript function is what I currently use to make the response usable in a web frontend (only that it doesn't work for cropped content). Ideally, it would be possible to have meilisearch respond in this format directly:

export type MatchPosition = { start: number; length: number };
export type MarkedText = 
  | { type: "standard"; text: string } 
  | { type: "highlighted"; text: string };

  export function highlightMatches(
    input: string,
    matches: MatchPosition[]
  ): MarkedText[] {
    const encoder = new TextEncoder();
    const decoder = new TextDecoder("utf-8");
    const byteArray = encoder.encode(input);
  
    const result: MarkedText[] = [];
    let currentByteIndex = 0;
  
    for (const { start, length } of matches) {
      // Add any text before the match as "standard"
      if (currentByteIndex < start) {
        result.push({
          type: "standard",
          text: decoder.decode(byteArray.slice(currentByteIndex, start)),
        });
      }
  
      // Add the matched text as "highlighted"
      result.push({
        type: "highlighted",
        text: decoder.decode(byteArray.slice(start, start + length)),
      });
  
      currentByteIndex = start + length;
    }
  
    // Add any remaining text after the last match as "standard"
    if (currentByteIndex < byteArray.length) {
      result.push({
        type: "standard",
        text: decoder.decode(byteArray.slice(currentByteIndex)),
      });
    }
  
    return result;
  }

[MatthiasvB]

Hi @irevoire, thanks for your response!

Regarding the security issue: It's quite simply that the absence of an alternative encourages people to use the HTML-prerendering feature. I don't know if you do any sanitation of the content before you insert HTML tags, but I don't think that would even make a lot of sense. But embedding HTML in user data that may also contain script tags and the like and rendering that in a browser is an XSS vulnerability. Beginners may not know this. And the lack of an alternative means that even experienced developers don't have an opportunity to avoid it. Hence, you don't have a vulnerability here, but you encourage building one in downstream code.

Regarding string indices: I don't know your internal considerations, but to me this looks like the leakage of an implementation detail (Rust) into the API (JSON). JavaScript Object Notation, however, clearly belongs in the technical realm of JS, which doesn't use byte indices. Hence, I expect that just about any consumer of your responses will be confused about this oddity.

Thank you for considering my thoughts

[irevoire]

I don't know if you do any sanitation of the content before you insert HTML tags, but I don't think that would even make a lot of sense.

Yeah, I see the idea. We don't sanitize anything, but the content is specified by the user doing the request. So I don't really see how that could result in an XSS 🤔
Basically, if an attacker is able to set the highlight pre-/post-tags, it means they're able to execute javascript on your computer and can already do whatever they want.

I don't really see where the vuln would be, but maybe I missed something 🤔

indices: I don't know your internal considerations, but to me this looks like the leakage of an implementation detail (Rust) into the API (JSON).

I don't know the historical reason for that but to me, it seems more performance related than anything, saying that the highlight starts at the byte X is O(1), saying that the highlight starts at the character X is O(X).
But most of the time I don't think people have really big fields, so that's a non-issue.

[MatthiasvB]

Yeah, I see the idea. We don't sanitize anything, but the content is specified by the user doing the request. So I don't really see how that could result in an XSS 🤔
Basically, if an attacker is able to set the highlight pre-/post-tags, it means they're able to execute javascript on your computer and can already do whatever they want.

I don't really see where the vuln would be, but maybe I missed something 🤔

This is not the mechanism I have in mind. Here's what I mean

1. I want to highlight the match in the cropped string. I can't use indices. I can only let meili insert HTML-Tags
2. I have to render the content as HTML now
3. In lots of cases, the content I'm rendering as HTML isn't actually created by me, but by one of my users
4. So I render user content + highlighting as HTML
5. A malicious user could have placed <script> tags in his content
6. So I render the user's malicious <script> as HTML, because I want highlighting
7. Depending on the use-case, that script is now running on any client that sees this content
8. I'm dealing with a huge shitstorm because my users just got hacked

[irevoire]

Ah ok I understood the issue you're describing.

• If you don't have control over your documents and let your user create documents
• And if you don't sanitize their documents
• And if you didn't set up your CORS correctly
• Then this mechanism could let a hacker extract information from your clients

Definitely not a cve in Meilisearch but yes, I agree it's not that niche and having another mechanism that don't rely on rendering the results is a good idea 👍
I don't know if and when it'll be prioritized but taking into account how much was cropped shouldn't be that hard, I believe.
imo most of the code should be in the meilisearch crate directly, you could try to take a look at the crates/meilisearch/routes/indexes/search.rs and look for the function that actually retrieves the content of the document. (the search request only returns the IDs of the documents initially)

[solonovamax]

And if you didn't set up your CORS correctly

Even with proper CORS set up, if you allow users to upload content that is served on the same domain then it may become an issue.

eg. The usecase I had when I made this request was for nextcloud.
Nextcloud allows users to upload arbitrary files, which can be fetched via a web request.
When documents are uploaded to nextcloud, they should not ever be sanitized, as they can be arbitrary things from executables to plain text/markdown to images.
Of course, only plain text content is being indexed here, but it can still be any textual content, such as an html tag which contains a link to another document they uploaded.
This would not be an issue on its own, however you can also share documents with another user, causing it to appear in their search results as well.
Although this might not be too bad on its own as you would require an account on the nextcloud instance. (however, if enabled, you can share documents with users on a different nextcloud instance through Federated Sharing)
But, if may allow a bad-actor to gain higher levels of permissions than they should have access to, for privilege escalation.
Like for example if a bad actor managed to compromise an employee's account in a company nextcloud server which used this, they could then upload malicious javascript to their own account so that if they create a share with the admin and the admin searches for something and it shows in the result, they could gain access to the admin's account, and possibly see if the admin account stores any passwords in their nextcloud account.

Whereas with the additional _matchesPosition key that I proposed, it would allow me to index into the string itself and ensure that it gets sanitized before I wrap it in my own tags for highlighting.

It's definitely not a CVE in meilisearch, but it can cause issues for users of meilisearch.