Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#7034 closed defect (fixed)

Possible SQL injection through _build_contact_where return results

Reported by: whatisthis Owned by:
Priority: normal Milestone: 0.15
Component: None Version:
Severity: normal Keywords: security
Cc: ioerror Blocked By:
Blocking: OS: All

Description

Bug description

In src/common/logger.py there is a method called _build_contact_where. This method is used to " Build the where clause for a jid, including metacontacts jid(s) if any". It maybe possible for the jid to include sql special characters. One such place where sql injection maybe possible is in the get_last_conversation_lines method where the following code appears:

def get_last_conversation_lines(... ... where_sql = self._build_contact_where(account, jid) ...

self.cur.execute(

SELECT time, kind, message FROM logs WHERE (%s) AND kind IN (%d, %d, %d, %d, %d) AND time > %d ORDER BY time DESC LIMIT %d OFFSET %d % (where_sql, constants.KIND_SINGLE_MSG_RECV,

constants.KIND_CHAT_MSG_RECV, constants.KIND_SINGLE_MSG_SENT, constants.KIND_CHAT_MSG_SENT, constants.KIND_ERROR, timed_out, restore_how_many_rows, pending_how_many)

)

Change History (8)

comment:1 Changed 4 years ago by asterix

I'm not expert in those security things, thanks for reporting that. Could a solution be that _build_contact_where() ruturns a string like "jid_id = ? OR jid_id = ? OR ..." and a list of jids [jid1, jid2] that will then be used in the get_last_conversation_lines function?

comment:2 Changed 4 years ago by whatisthis

Why don't you just use a prepared statement?

comment:3 Changed 4 years ago by asterix

Because I don't know what that is for the moment.

comment:4 Changed 4 years ago by whatisthis

  • Keywords security added

comment:5 Changed 4 years ago by asterix

I looked, a prepared statement is my first proposition, using ?. Going to implement that.

comment:6 Changed 4 years ago by Yann Leboulanger <asterix@…>

  • Milestone set to 0.15
  • Resolution set to fixed
  • Status changed from new to closed

(In [bfd5f94489d8]) use prepared statements in all SQL queries that contains jids to prevent SQL injection. Fixes #7034

comment:7 Changed 4 years ago by Yann Leboulanger <asterix@…>

(In [988e38ce0e0c]) use prepared statements in all SQL queries that contains jids to prevent SQL injection. Fixes #7034

comment:8 Changed 4 years ago by thijs

This is CVE-2012-2086.

Note: See TracTickets for help on using tickets.