Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetCachedFileContents has hidden DDOS functionality #17

Closed
kowach opened this issue May 20, 2015 · 4 comments
Closed

GetCachedFileContents has hidden DDOS functionality #17

kowach opened this issue May 20, 2015 · 4 comments

Comments

@kowach
Copy link

kowach commented May 20, 2015

Can you fix PVRIptvData::GetCachedFileContents so it does not stat files that are on web and put some global error counter so it does not infinitely DDOS servers for XMLTV epg or for channel logos?

Let me explain, there are bunch of people who entered wrong XMLTV URL-s or URL-s which are not working anymore. Those URL-s stay in config and iptvsimple continues to download it for every channel every five minutes, and if there is a error it tries 2 more times. And it is doing that all day because Kodis are online nonstop (home media pc, R-pi,..).

In PVRIptvData.cpp LoadEPG(iStart, iEnd) is called for every channel and it calls GetCachedFileContents every time. If I have 500 channels, that is 500 GetCachedFileContents. But there is only one XML EPG file. No need to call it 500 times, only 1 is ok.

If you call XBMC->StatFile(strFilePath.c_str(), &statOrig); on URL, it will send HEAD request to server to see if file was changes. Server are usually missconfigured or URL is wrong. So I would make this quick fix (I'm not a C programmer, but you will get the idea):

if (bUseCache && XBMC->FileExists(strCachedPath.c_str(), false)) 
{
   struct __stat64 statCached;
   struct __stat64 statOrig;

   XBMC->StatFile(strCachedPath.c_str(), &statCached);
   XBMC->StatFile(strFilePath.c_str(), &statOrig);

   bNeedReload = statCached.st_mtime < statOrig.st_mtime || statOrig.st_mtime == 0;
} 

to

if (bUseCache && XBMC->FileExists(strCachedPath.c_str(), false)) 
{
   struct __stat64 statCached;
   struct __stat64 statOrig;

   XBMC->StatFile(strCachedPath.c_str(), &statCached);

   int a = (int)strFilePath.Find('://');
   if( strFilePath.substr(0, a)=='http' || strFilePath.substr(0, a)=='https')  // or ftp?
   {
        // Don't download if cache file is newer than 2 hours
        // I don't know how to get current_time?, please fix this
        bNeedReload = statCached.st_mtime + 7200 < current_time;
   }
   else
   {
       XBMC->StatFile(strFilePath.c_str(), &statOrig);

       bNeedReload = statCached.st_mtime < statOrig.st_mtime || statOrig.st_mtime == 0;
   }
} 

This will solve problem of sending hundreds of HEAD requests to server every 5 minutes.

But the problem stays if server responses with error 404 or 503 or no response. File will not be cached. So GetFileContents should remember failed URL-s and not use them for next day or more.

@kowach
Copy link
Author

kowach commented May 20, 2015

An this function HadErrorsInLast24hours would fix second problem:

if (bNeedReload) 
{
    // I would add this check: if it had more than 3 failed attempts (error 4xx or 5xx or empty response) in last 24 hours
    if( HadErrorsInLast24hours(strFilePath) ) return false;

    GetFileContents(strFilePath, strContents);

    // write to cache
    if (bUseCache && strContents.length() > 0) 
    {
        void* fileHandle = XBMC->OpenFileForWrite(strCachedPath, true);
        if (fileHandle)
        {
            XBMC->WriteFile(fileHandle, strContents.c_str(), strContents.length());
            XBMC->CloseFile(fileHandle);
        }
    }
    return strContents.length();
} 

my frustration can be seen on http://forum.kodi.tv/showthread.php?tid=227177&pid=2006157

@afedchin
Copy link
Contributor

In PVRIptvData.cpp LoadEPG(iStart, iEnd) is called for every channel and it calls GetCachedFileContents every time. If I have 500 channels, that is 500 GetCachedFileContents. But there is only one XML EPG file. No need to call it 500 times, only 1 is ok.

Latest changes changed this behavior and now LoadEPG(iStart, iEnd) is called once if the interval is changed. i.e only once for first channel - https://github.com/kodi-pvr/pvr.iptvsimple/blob/master/src/PVRIptvData.cpp#L661. use the latest version of the addon please.

Server are usually missconfigured

this is server's owner problem

or URL is wrong.

I can not trigger a user to change the url. But url will opened only once for first channel as I explained above.

This will solve problem of sending hundreds of HEAD requests to server every 5 minutes.

the problem of 5 mins can be solved via advancedsettings.xml. But I agree with you what there is a problem in the PVRManager which try to update EPG every 5 mins for every channel which has no EPG data. But this is problem of PVR manager not of the addon. solving problem of PVR manager into addon is wrong.

So GetFileContents should remember failed URL-s and not use them for next day or more.

why not a week? If URL is not accessible during five minutes (server reboots as example) the user may not have EPG during day or two?

@kowach
Copy link
Author

kowach commented May 20, 2015

I have tested on 1.9.11 windows version and it sends HEAD request to server for every channel nonstop. If this is fixed. Great.

People who are using Kodi are not so smart so we have to handle those cases. Most of request to my server is from wrong URL path.

If URL is not accessible during five minutes you double check interval, 10 minutes, 20 minutes, 40 and so on...

@Jalle19
Copy link
Contributor

Jalle19 commented Jul 22, 2015

Closing this since it shouldn't be an issue anymore.

@Jalle19 Jalle19 closed this as completed Jul 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants