🔒 [security fix] Fix Solr Query Injection in solrHelper.ts #772
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,8 @@ | ||
| const escapeSolrQuery = (query: any) => { | ||
| if (typeof query !== 'string') query = query ? String(query) : '' | ||
| return query.replace(/([+\-!(){}\[\]^"~*?:\/\\|&])/g, "\\$1"); | ||
|
Check failure on line 3 in src/utils/solrHelper.ts
|
||
| } | ||
|
|
||
| const prepareOrderQuery = (params: any) => { | ||
| const viewSize = params.viewSize ? params.viewSize : process.env.VUE_APP_VIEW_SIZE; | ||
| const viewIndex = params.viewIndex ? params.viewIndex : 0; | ||
|
|
@@ -15,80 +20,80 @@ | |
| "start": viewIndex * viewSize | ||
| }, | ||
| "query":"(*:*)", | ||
| "filter": [`docType: ${params.docType ? escapeSolrQuery(params.docType) : 'OISGIR'}`] | ||
| } | ||
| } as any | ||
|
|
||
| if (params.queryString) { | ||
| payload.json.query = `(*${escapeSolrQuery(params.queryString)}*)` | ||
| payload.json.params['qf'] = "productId productName virtualProductName orderId productSku customerId customerName customerPartyName search_customerPartyName search_orderIdentifications goodIdentifications" | ||
| payload.json.params['defType'] = "edismax" | ||
| } | ||
|
|
||
| if (params.shipmentMethodTypeId) { | ||
| payload.json.filter.push(`shipmentMethodTypeId: ${escapeSolrQuery(params.shipmentMethodTypeId)}`) | ||
| } | ||
|
|
||
| if (params.orderTypeId) { | ||
| payload.json.filter.push(`orderTypeId: ${escapeSolrQuery(params.orderTypeId)}`) | ||
| } | ||
|
|
||
| if (params.orderStatusId) { | ||
| payload.json.filter.push(`orderStatusId: ${escapeSolrQuery(params.orderStatusId)}`) | ||
| } | ||
|
|
||
| if (params['-shipmentStatusId']) { | ||
| payload.json.filter.push(`-shipmentStatusId: ${escapeSolrQuery(params['-shipmentStatusId'])}`) | ||
| } | ||
|
|
||
| if (params.shipmentStatusId) { | ||
| payload.json.filter.push(`shipmentStatusId: ${escapeSolrQuery(params.shipmentStatusId)}`) | ||
| } | ||
|
|
||
| if (params['-fulfillmentStatus']) { | ||
| payload.json.filter.push(`-fulfillmentStatus: ${escapeSolrQuery(params['-fulfillmentStatus'])}`) | ||
|
dt2patel marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Applying There was a problem hiding this comment. I have reviewed the changes. I also fixed two ESLint 'no-useless-escape' errors in the
Contributor
Author
There was a problem hiding this comment. @jules Applying escapeSolrQuery to the '-fulfillmentStatus' parameter will break existing functionality. In src/store/modules/order/actions.ts, this parameter is passed as a complex Solr expression (Cancelled OR Rejected OR Completed). Escaping the parentheses will cause Solr to treat them as literal characters, leading to incorrect filtering. This utility should only be applied to values that are intended to be single terms, or the application should be refactored to separate structural syntax from user-provided values. |
||
| } | ||
|
|
||
| if (params.facilityId) { | ||
| payload.json.filter.push(`facilityId: ${escapeSolrQuery(params.facilityId)}`) | ||
| } | ||
|
|
||
| if (params.productId) { | ||
| payload.json.filter.push(`productId: ${escapeSolrQuery(params.productId)}`) | ||
| } | ||
|
|
||
| if (params.filters) { | ||
| Object.keys(params.filters).forEach((key) => { | ||
| payload.json.filter.push(`${escapeSolrQuery(key)}: ${escapeSolrQuery(params.filters[key])}`); | ||
| }); | ||
| } | ||
|
|
||
| if(params.orderIds){ | ||
| payload.json.filter.push(`orderId: (${params.orderIds.map((orderId: string) => escapeSolrQuery(orderId)).join(' OR ')})`) | ||
| } | ||
|
|
||
| if(params.facet) { | ||
| payload.json['facet'] = params.facet | ||
| } | ||
|
|
||
| if (params.orderPartSeqId) { | ||
| payload.json.filter.push(`shipGroupSeqId: ${escapeSolrQuery(params.orderPartSeqId)}`) | ||
| } | ||
|
|
||
| if (params.orderId) { | ||
| payload.json.filter.push(`orderId: ${escapeSolrQuery(params.orderId)}`) | ||
| } | ||
|
|
||
| if (params.shipGroupSeqId) { | ||
| payload.json.filter.push(`shipGroupSeqId: ${escapeSolrQuery(params.shipGroupSeqId)}`) | ||
| } | ||
|
|
||
| if (params['-shipGroupSeqId']) { | ||
| payload.json.filter.push(`-shipGroupSeqId: ${escapeSolrQuery(params['-shipGroupSeqId'])}`) | ||
| } | ||
|
|
||
| if(params.orderItemStatusId) { | ||
| payload.json.filter.push(`orderItemStatusId: ${escapeSolrQuery(params.orderItemStatusId)}`) | ||
| } | ||
|
|
||
| return payload | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation of
escapeSolrQueryincorrectly converts falsy values like0orfalseto an empty string. This can lead to incorrect Solr queries when filtering by these values. Using nullish coalescing andtoString()ensures that0andfalseare correctly preserved as strings while handlingnullandundefined.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jules verify and correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jules The current implementation of escapeSolrQuery incorrectly converts falsy values like 0 or false to an empty string. This can lead to incorrect Solr queries when filtering by these values. Using nullish coalescing and toString() ensures that 0 and false are correctly preserved as strings while handling null and undefined.