<p>Just looked quickly a bit ...</p>
<p>I spotted some sprintf() which are unsafe, especially when dealing with DB string or blobs, the target buffer is 255 bytes in size, but I haven't seen any check of input size.</p>
<p>This snippet needs some checks as well, for allocated pointer and snprintf:</p>
<pre><code>+                        int username_size=VAL_STR(tval).len+1*sizeof(char);
+                       username = (char*)pkg_malloc(username_size);
+                       snprintf(username,username_size,"%s",VAL_STR(tval).s);
</code></pre>
<p><code>pkg_strdup()</code> can return NULL, but that is not checked -- although, I didn't looked more to see if it always safe to work further if the return is NULL there.</p>
<p>I will ask to see if anyone else can do additional work to review. If not, as I said, I do not have anything against merging it.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/kamailio/kamailio/pull/1446#issuecomment-404219576">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AF36ZRnMSWiFGQ3kgyJMSkAS03qWM7icks5uFh7pgaJpZM4SJS1k">mute the thread</a>.<img src="https://github.com/notifications/beacon/AF36ZdwqYu682zXHjRr3JimZTobA506Dks5uFh7pgaJpZM4SJS1k.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@miconda in #1446: Just looked quickly a bit ...\r\n\r\nI spotted some sprintf() which are unsafe, especially when dealing with DB string or blobs, the target buffer is 255 bytes in size, but I haven't seen any check of input size.\r\n\r\nThis snippet needs some checks as well, for allocated pointer and snprintf:\r\n\r\n```\r\n+\t\t\tint username_size=VAL_STR(tval).len+1*sizeof(char);\r\n+\t\t\tusername = (char*)pkg_malloc(username_size);\r\n+\t\t\tsnprintf(username,username_size,\"%s\",VAL_STR(tval).s);\r\n```\r\n\r\n`pkg_strdup()` can return NULL, but that is not checked -- although, I didn't looked more to see if it always safe to work further if the return is NULL there.\r\n\r\nI will ask to see if anyone else can do additional work to review. If not, as I said, I do not have anything against merging it."}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1446#issuecomment-404219576"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/kamailio/kamailio/pull/1446#issuecomment-404219576",
"url": "https://github.com/kamailio/kamailio/pull/1446#issuecomment-404219576",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
},
{
"@type": "MessageCard",
"@context": "http://schema.org/extensions",
"hideOriginalBody": "false",
"originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB",
"title": "Re: [kamailio/kamailio] Modules: db_redisusrloc for usrloc module (#1446)",
"sections": [
{
"text": "",
"activityTitle": "**Daniel-Constantin Mierla**",
"activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png",
"activitySubtitle": "@miconda",
"facts": [

]
}
],
"potentialAction": [
{
"name": "Add a comment",
"@type": "ActionCard",
"inputs": [
{
"isMultiLine": true,
"@type": "TextInput",
"id": "IssueComment",
"isRequired": false
}
],
"actions": [
{
"name": "Comment",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"kamailio/kamailio\",\n\"issueId\": 1446,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}"
}
]
},
{
"name": "Close pull request",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"kamailio/kamailio\",\n\"pullRequestId\": 1446\n}"
},
{
"targets": [
{
"os": "default",
"uri": "https://github.com/kamailio/kamailio/pull/1446#issuecomment-404219576"
}
],
"@type": "OpenUri",
"name": "View on GitHub"
},
{
"name": "Unsubscribe",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 304426340\n}"
}
],
"themeColor": "26292E"
}
]</script>